[Cluster-devel] cluster/cmirror/src functions.c

[jbrassow@sourceware.org <jbrassow@sourceware.org>]

CVSROOT:	/cvs/cluster
Module name:	cluster
Branch: 	RHEL5
Changes by:	jbrassow at sourceware.org	2008-02-13 15:06:17

Modified files:
	cmirror/src    : functions.c 

Log message:
	- change the way 'is_remote_recovering' works to improve overall
	performance.
	
	Before a mirror issues a write, it must call 'is_remote_recovering'
	to ensure that another machine will not be recovering the region during
	the write.  This function can dramatically slow things down.  One way to
	increase performance is to note when the mirror is in-sync - then
	is_remote_recovering can return 0 without having to send the request
	around the cluster.  (This has already been done.)  This greatly speeds up
	I/O during nominal mirror operation.  However, I/O during mirror resyncing
	is still greatly reduced.  The problem is that the cluster network is
	consumed with handling 'is_remote_recovering' calls that it becomes hard
	to actually do the recovery.  The fix is to only allow one
	is_remote_recovering call to go to the cluster every 1/4 sec.  When the
	call goes up to userspace, it also retrieves info about how far along
	the resync is.  If a request is determined to already be in sync by that
	info, then the region is not recovering and can safely be answered without
	having to send the request on to the cluster.  This approach has greatly
	improved both the recovery and nominal throughput.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/cmirror/src/functions.c.diff?cvsroot=cluster&only_with_tag=RHEL5&r1=1.1.2.14&r2=1.1.2.15

--- cluster/cmirror/src/Attic/functions.c	2008/02/08 14:30:10	1.1.2.14
+++ cluster/cmirror/src/Attic/functions.c	2008/02/13 15:06:17	1.1.2.15
@@ -1359,8 +1359,8 @@
  */
 static int clog_is_remote_recovering(struct clog_tfr *tfr)
 {
-	int *rtn = (int *)tfr->data;
 	uint64_t region = *((uint64_t *)(tfr->data));
+	struct { int is_recovering; uint64_t sync_search; } *pkg = (void *)tfr->data;
 	struct log_c *lc = get_log(tfr->uuid);
 
 	if (!lc)
@@ -1372,18 +1372,21 @@
 	if (lc->recovery_halted) {
 		LOG_DBG("[%s] Recovery halted... [not remote recovering]: %llu",
 			SHORT_UUID(lc->uuid), (unsigned long long)region);
-		*rtn = 0;
+		pkg->is_recovering = 0;
+		pkg->sync_search = lc->region_count; /* none are recovering */
 	} else {
-		*rtn = !log_test_bit(lc->sync_bits, region);
+		pkg->is_recovering = !log_test_bit(lc->sync_bits, region);
+		pkg->sync_search = lc->sync_search;
 		LOG_DBG("[%s] Region is %s: %llu",
 			SHORT_UUID(lc->uuid),
 			(region == lc->recovering_region) ?
 			"currently remote recovering" :
-			(*rtn) ? "pending remote recovery" :
+			(pkg->is_recovering) ? "pending remote recovery" :
 			"not remote recovering", (unsigned long long)region);
 	}
 
-	if (*rtn && (region != lc->recovering_region)) {
+	if (pkg->is_recovering &&
+	    (region != lc->recovering_region)) {
 		struct recovery_request *rr;
 
 		/* Already in the list? */
@@ -1404,7 +1407,8 @@
 	}
 
 out:
-	tfr->data_size = sizeof(*rtn);
+
+	tfr->data_size = sizeof(*pkg);
 
 	return 0;	
 }