[Fedora-directory-commits] ldapserver/ldap/servers/plugins/replication repl5_inc_protocol.c, 1.11, 1.12

[Richard Allen Megginson (rmeggins)]

Author: rmeggins

Update of /cvs/dirsec/ldapserver/ldap/servers/plugins/replication
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv18524/bug450973/ldap/servers/plugins/replication

Modified Files:
	repl5_inc_protocol.c 
Log Message:
Resolves: bug 450973
Bug Description: rhds80 account accountunlocktime attribute breaks replication
Reviewed by: nhosoi (Thanks!)
Fix Description: We were not handling errors returned from the consumer correctly in the async replication code.  The problem was that we were exiting the async read results thread immediately.  However, we needed to wait for and read all of the outstanding responses, then exit the thread when all of them had been read.  The new code handles this case correctly, allowing us to read all of the pending responses before exiting.

The flip side of this is that passwordIsGlobalPolicy only works on the _consumer_.  It has no effect whatsoever on the _supplier_ side of replication.  The fix for this is to configure fractional replication _always_ and to add the password policy op attrs to the list of attrs not to replicate.  This should work fine with RHDS 8.0.0-14 and later.
Platforms tested: RHEL5
Flag Day: no
Doc impact: Yes.  We will need to document exactly how passwordIsGlobalPolicy works and how to configure fractional replication.
QA impact: Will need to do more testing of MMR with account lockout to make sure this error does not blow up MMR anymore.
New Tests integrated into TET: Working on it.



Index: repl5_inc_protocol.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl5_inc_protocol.c,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- repl5_inc_protocol.c	18 Oct 2007 00:08:31 -0000	1.11
+++ repl5_inc_protocol.c	23 Jun 2008 18:38:40 -0000	1.12
@@ -294,6 +294,7 @@
 		while (!finished)
 		{
 			conres = conn_read_result_ex(conn, NULL, NULL, NULL, &message_id, 0);
+			slapi_log_error(SLAPI_LOG_REPL, NULL, "repl5_inc_result_threadmain: read result for message_id %d\n", message_id);
 			/* Timeout here means that we didn't block, not a real timeout */
 			if (CONN_TIMEOUT == conres)
 			{
@@ -328,6 +329,7 @@
 		}
 		if (conres != CONN_TIMEOUT)
 		{
+			int should_finish = 0;
 			if (message_id) 
 			{
 				rd->last_message_id_received = message_id;
@@ -344,16 +346,18 @@
 			}
 
 			conn_get_error_ex(conn, &operation_code, &connection_error, &ldap_error_string);
-			slapi_log_error(SLAPI_LOG_REPL, NULL, "repl5_inc_result_threadmain: result %d, %d, %d, %s\n", operation_code,connection_error,conres,ldap_error_string);
-			rd->result = repl5_inc_update_from_op_result(rd->prp, conres, connection_error, csn_str, uniqueid, replica_id, &finished, &(rd->num_changes_sent));
-			if (rd->result)
+			slapi_log_error(SLAPI_LOG_REPL, NULL, "repl5_inc_result_threadmain: result %d, %d, %d, %d, %s\n", operation_code,connection_error,conres,message_id,ldap_error_string);
+			rd->result = repl5_inc_update_from_op_result(rd->prp, conres, connection_error, csn_str, uniqueid, replica_id, &should_finish, &(rd->num_changes_sent));
+			if (rd->result || should_finish)
 			{
-				slapi_log_error(SLAPI_LOG_REPL, NULL, "repl5_inc_result_threadmain: got op result %d\n", rd->result);
+				slapi_log_error(SLAPI_LOG_REPL, NULL, "repl5_inc_result_threadmain: got op result %d should finish %d\n", rd->result, should_finish);
 				/* If so then we need to take steps to abort the update process */
 				PR_Lock(rd->lock);
 				rd->abort = 1;
 				PR_Unlock(rd->lock);
 				/* We also need to log the error, including details stored from when the operation was sent */
+				/* we cannot finish yet - we still need to waitfor the pending results, then
+				   the main repl code will shut down this thread */
 			}
 		}
 		/* Should we stop ? */