Trac Ticket #359 - Database RUV could mismatch the one

     in changelog under the stress

https://fedorahosted.org/389/ticket/359

Fix Description:
. Fix for llistRemoveCurrentAndGetNext (llist.c) in commit
  f0f74b57f81998a325dc7472b9ea9b44c5ff6439 was not correct.
. Undo the changes made in ldbm_back_add, ldbm_back_modrdn
  (ldbm_add.c, ldbm_modrdn.c) in commit
  f0f74b57f81998a325dc7472b9ea9b44c5ff6439.
. urp.c: In the original design, if op_result is LDA_SUCCESS and
  the return code is -1, the operation is skipped, but the max
  csn is updated in RUV.  Currently, we have no way to do so.
  This patch eliminates the cases: 2 in urp_add_operation and 1
  in urp_modrdn_operation are now LDAP_UNWILLING_TO_PERFORM; 1 in
  urp_modrdn_operation (the existing entry is the same as the
  target one) returns 0.

ldap/servers/plugins/replication/llist.c

@@ -165,14 +165,14 @@ void* llistRemoveCurrentAndGetNext (LList *list, void **iterator)
 	if (node)
 	{
 		prevNode->next = node->next;	
+		if (list->tail == node) {
+			list->tail = prevNode;
+		}
 		_llistDestroyNode (&node, NULL);
 		node = prevNode->next;
 		if (node) {
 			return node->data;
 		} else {
-			if (list->head->next == NULL) {
-				list->tail = NULL;
-			}
 			return NULL;
 		}
 	}

ldap/servers/plugins/replication/urp.c

@@ -137,7 +137,17 @@ urp_add_operation( Slapi_PBlock *pb )
 		 * - It could be a replay of the same Add, or
 		 * - It could be a UUID generation collision, or
 		 */
-		op_result = LDAP_SUCCESS;
+		/* 
+		 * This operation won't be replayed.  That is, this CSN won't update
+		 * the max csn in RUV. The CSN is left uncommitted in RUV unless an
+		 * error is set to op_result.  Just to get rid of this CSN from RUV,
+		 * setting an error to op_result
+		 */
+		/* op_result = LDAP_SUCCESS; */
+		slapi_log_error(slapi_log_urp, sessionid,
+		          "urp_add (%s): an entry with this uniqueid already exists.\n",
+		          slapi_entry_get_dn_const(existing_uniqueid_entry));
+		op_result= LDAP_UNWILLING_TO_PERFORM;
 		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result);
 		rc= -1; /* Ignore this Operation */
 		PROFILE_POINT; /* Add Conflict; UniqueID Exists;  Ignore */
@@ -249,7 +259,17 @@ urp_add_operation( Slapi_PBlock *pb )
 		 * b) We've seen the Operation before.
 		 * Let's go with (b) and ignore the little bastard.
 		 */
-		op_result= LDAP_SUCCESS;
+		/* 
+		 * This operation won't be replayed.  That is, this CSN won't update
+		 * the max csn in RUV. The CSN is left uncommitted in RUV unless an
+		 * error is set to op_result.  Just to get rid of this CSN from RUV,
+		 * setting an error to op_result
+		 */
+		/* op_result = LDAP_SUCCESS; */
+		slapi_log_error(slapi_log_urp, sessionid,
+		"urp_add (%s): The CSN of the Operation and the Entry DN are the same.",
+		slapi_entry_get_dn_const(existing_dn_entry));
+		op_result= LDAP_UNWILLING_TO_PERFORM;
 		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result);
 		rc= -1; /* Ignore this Operation */
 		PROFILE_POINT; /* Add Conflict; Entry Exists; Same CSN */
@@ -311,7 +331,17 @@ urp_modrdn_operation( Slapi_PBlock *pb )
 		 * The Operation CSN is not newer than the DN CSN.
 		 * Either we're beaten by another ModRDN or we've applied the op.
 		 */
-		op_result= LDAP_SUCCESS;
+		/* op_result= LDAP_SUCCESS; */
+		/* 
+		 * This operation won't be replayed.  That is, this CSN won't update
+		 * the max csn in RUV. The CSN is left uncommitted in RUV unless an
+		 * error is set to op_result.  Just to get rid of this CSN from RUV,
+		 * setting an error to op_result
+		 */
+		slapi_log_error(slapi_log_urp, sessionid,
+		           "urp_modrdn (%s): operation CSN is newer than the DN CSN.\n",
+		           slapi_entry_get_dn_const(target_entry));
+		op_result= LDAP_UNWILLING_TO_PERFORM;
 		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result);
 		rc= -1; /* Ignore the modrdn */
 		PROFILE_POINT; /* ModRDN Conflict; Entry with Target DN Exists; OPCSN is not newer. */
@@ -323,8 +353,8 @@ urp_modrdn_operation( Slapi_PBlock *pb )
 	/* newrdn is no need to be case-ignored (get_rdn_plus_uniqueid) */
 	slapi_pblock_get(pb, SLAPI_MODRDN_NEWRDN, &newrdn);
 	slapi_pblock_get(pb, SLAPI_TARGET_UNIQUEID, &op_uniqueid);
-   	slapi_pblock_get(pb, SLAPI_MODRDN_PARENT_ENTRY, &parent_entry);
-   	slapi_pblock_get(pb, SLAPI_MODRDN_NEWPARENT_ENTRY, &new_parent_entry);
+	slapi_pblock_get(pb, SLAPI_MODRDN_PARENT_ENTRY, &parent_entry);
+	slapi_pblock_get(pb, SLAPI_MODRDN_NEWPARENT_ENTRY, &new_parent_entry);
 	slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &newsuperior);
 
 	if ( is_tombstone_entry (target_entry) )
@@ -361,26 +391,29 @@ urp_modrdn_operation( Slapi_PBlock *pb )
 		goto bailout;
 	}
 
-   	slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &existing_entry);
-    if(existing_entry!=NULL) 
+	slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &existing_entry);
+	if(existing_entry!=NULL) 
 	{
 	    /*
 	     * An entry with the target DN already exists.
-		 * The smaller dncsn wins. The loser changes its RDN to
-		 * uniqueid+baserdn, and adds operational attribute
-		 * ATTR_NSDS5_REPLCONFLIC
+	     * The smaller dncsn wins. The loser changes its RDN to
+	     * uniqueid+baserdn, and adds operational attribute
+	     * ATTR_NSDS5_REPLCONFLIC
 	     */
 
 		existing_uniqueid = slapi_entry_get_uniqueid (existing_entry);
 		existing_sdn = slapi_entry_get_sdn_const ( existing_entry);
 
 		/*
-		 * Dismiss the operation if the existing entry is the same as the target one.
+		 * It used to dismiss the operation if the existing entry is 
+		 * the same as the target one.
+		 * But renaming the RDN with the one which only cases are different,
+		 * cn=ABC --> cn=Abc, this case matches.  We should go forward the op.
 		 */
 		if (strcmp(op_uniqueid, existing_uniqueid) == 0) {
 			op_result= LDAP_SUCCESS;
 			slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result);
-			rc = -1; /* Ignore the op */
+			rc = 0; /* Don't ignore the op */
 			PROFILE_POINT; /* ModRDN Replay */
 			goto bailout;
 		}

ldap/servers/slapd/back-ldbm/ldbm_add.c

@@ -117,7 +117,6 @@ ldbm_back_add( Slapi_PBlock *pb )
 	int is_ruv = 0;				 /* True if the current entry is RUV */
 	CSN *opcsn = NULL;
 	entry_address addr = {0};
-	int val = 0;
 
 	slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li );
 	slapi_pblock_get( pb, SLAPI_ADD_ENTRY, &e );
@@ -1044,26 +1043,26 @@ error_return:
 		disk_full = 1;
 	}
 
-	/* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */
-	slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val);
-	if (!val) {
-		if (!ldap_result_code) {
-			ldap_result_code = LDAP_OPERATIONS_ERROR;
-		}
-		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code);
-	}
-	slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val );
-	if (!val) {
-		val = -1;
-		slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &val );
-	}
-
 diskfull_return:
 	if (disk_full) {
 		rc= return_on_disk_full(li);
 	} else {
 		/* It is safer not to abort when the transaction is not started. */
 		if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) {
+			/* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */
+			int val = 0;
+			slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val);
+			if (!val) {
+				if (!ldap_result_code) {
+					ldap_result_code = LDAP_OPERATIONS_ERROR;
+				}
+				slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code);
+			}
+			slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val );
+			if (!val) {
+				val = -1;
+				slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &val );
+			}
 			/* call the transaction post add plugins just before the abort */
 			if ((retval = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_TXN_POST_ADD_FN))) {
 				int opreturn = 0;

ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

@@ -120,7 +120,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
     const char *newdn = NULL;
     char *newrdn = NULL;
     int opreturn = 0;
-    int val = 0;
 
     /* sdn & parentsdn need to be initialized before "goto *_return" */
     slapi_sdn_init(&dn_newdn);
@@ -1199,20 +1198,6 @@ error_return:
         disk_full = 1;
     }
 
-    /* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */
-    slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val);
-    if (!val) {
-        if (!ldap_result_code) {
-            ldap_result_code = LDAP_OPERATIONS_ERROR;
-        }
-        slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code);
-    }
-    slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val );
-    if (!val) {
-        opreturn = -1;
-        slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &opreturn );
-    }
-
     if (disk_full) 
     {
         retval = return_on_disk_full(li);
@@ -1222,6 +1207,20 @@ error_return:
         /* It is safer not to abort when the transaction is not started. */
         if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn))
         {
+            /* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */
+            int val = 0;
+            slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val);
+            if (!val) {
+                if (!ldap_result_code) {
+                    ldap_result_code = LDAP_OPERATIONS_ERROR;
+                }
+                slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code);
+            }
+            slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val );
+            if (!val) {
+                opreturn = -1;
+                slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &opreturn );
+            }
             /* call the transaction post modrdn plugins just before the commit */
             if ((retval = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN))) {
                 LDAPDebug1Arg( LDAP_DEBUG_TRACE, "SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN plugin "