Ticket #383 - usn + mmr = deletions are not replicated

https://fedorahosted.org/389/ticket/383
Resolves: Ticket #383
Bug Description: usn + mmr = deletions are not replicated
Reviewed by: mreynolds (Thanks!)
Branch: master
Fix Description: The problem was that because usn was creating a tombstone,
it was also setting the OP_FLAG_TOMBSTONE flag in the operation, which was
causing the operation to think it was deleting a tombstone.  The fix is to
not set this flag, and instead have operations that delete tombstones to
set that flag explicitly when creating the delete op request.
In addition, the CSN for delete ops was not being logged - the usn bepostop
was deleting it, even when replication was being used.  Previously the csn
was needed as a "trigger" to tell the ldbm_back_delete code to create a
tombstone rather than deleting the entry outright.  Instead, use the
 slapi_operation_get_replica_attr (pb, operation,
                                   "nsds5ReplicaTombstonePurgeInterval,
                                   &create_tombstone_entry)
to determine whether or not to create a tombstone entry.  Both replication
and usn configure this, so if using one or both of those, tombstones will
be created, otherwise, not.
Platforms tested: RHEL6 x86_64, Fedora 17
Flag Day: no
Doc impact: no
(cherry picked from commit 73e077189820130c93e25e359d5935794dbbf3ee)

ldap/servers/plugins/usn/usn.c

@@ -45,8 +45,6 @@ static Slapi_PluginDesc pdesc = {
         "USN", VENDOR, DS_PACKAGE_VERSION,
         "USN (Update Sequence Number) plugin" };
 
-static CSNGen *_usn_csngen = NULL;
-
 static void *_usn_identity = NULL;
 
 static int usn_preop_init(Slapi_PBlock *pb);
@@ -152,13 +150,6 @@ usn_preop_init(Slapi_PBlock *pb)
 {
     int rc = 0;
     int predel = SLAPI_PLUGIN_PRE_DELETE_FN;
-    /* set up csn generator for tombstone */
-    _usn_csngen = csngen_new(USN_CSNGEN_ID, NULL);
-    if (NULL == _usn_csngen) {
-        slapi_log_error(SLAPI_LOG_FATAL, USN_PLUGIN_SUBSYSTEM,
-                        "usn_preop_init: csngen_new failed\n");
-        rc = -1;
-    }
 
     if (slapi_pblock_set(pb, predel, (void *)usn_preop_delete) != 0) {
         slapi_log_error(SLAPI_LOG_FATAL, USN_PLUGIN_SUBSYSTEM,
@@ -302,15 +293,11 @@ bail:
     return rc;
 }
 
-/*
- * usn_close: release the csn generator used to convert an entry to tombstone
- */
 static int
 usn_close(Slapi_PBlock *pb)
 {
     slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM, "--> usn_close\n");
 
-    csngen_free(&_usn_csngen);
     g_plugin_started = 0;
 
     slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM, "<-- usn_close\n");
@@ -325,32 +312,14 @@ static int
 usn_preop_delete(Slapi_PBlock *pb)
 {
     int rc = 0;
-    CSN *csn = NULL;
-    CSN *orig_csn = NULL;
     Slapi_Operation *op = NULL;
 
     slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM,
                     "--> usn_preop_delete\n");
 
     slapi_pblock_get(pb, SLAPI_OPERATION, &op);
-    orig_csn = operation_get_csn(op);
-
-    if (NULL == orig_csn) {
-        /* 
-         * No other plugins hasn't set csn yet, so let's set USN's csn.
-         * If other plugin overrides csn and replica_attr_handler, that's fine.
-         */
-        rc = csngen_new_csn(_usn_csngen, &csn, PR_FALSE /* notify */);
-        if (CSN_SUCCESS != rc) {
-            slapi_log_error(SLAPI_LOG_FATAL, USN_PLUGIN_SUBSYSTEM,
-                            "usn_preop_delete: csngen_new failed (%d)\n", rc);
-            csn_free(&csn);
-            goto bail;
-        }
-        operation_set_csn(op, csn);
-        slapi_operation_set_replica_attr_handler(op, (void *)usn_get_attr);
-    }
-bail:
+    slapi_operation_set_replica_attr_handler(op, (void *)usn_get_attr);
+
     slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM,
                     "<-- usn_preop_delete\n");
 
@@ -486,11 +455,6 @@ usn_betxnpreop_delete(Slapi_PBlock *pb)
         rc = LDAP_PARAM_ERROR;    
         goto bail;
     }
-    if (e->e_flags & SLAPI_ENTRY_FLAG_TOMBSTONE) {
-        Slapi_Operation *op = NULL;
-        slapi_pblock_get(pb, SLAPI_OPERATION, &op);
-        slapi_operation_set_flag(op, OP_FLAG_TOMBSTONE_ENTRY);
-    }
     _usn_add_next_usn(e, be);
 bail:
     slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM,
@@ -615,17 +579,10 @@ usn_bepostop_delete (Slapi_PBlock *pb)
 {
     int rc = -1;
     Slapi_Backend *be = NULL;
-    Slapi_Operation *op = NULL;
-    CSN *csn = NULL;
 
     slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM,
                     "--> usn_bepostop\n");
 
-    slapi_pblock_get(pb, SLAPI_OPERATION, &op);
-    csn = operation_get_csn(op);
-    csn_free(&csn);
-    operation_set_csn(op, NULL);
-
     /* if op is not successful, don't increment the counter */
     slapi_pblock_get(pb, SLAPI_RESULT_CODE, &rc);
     if (LDAP_SUCCESS != rc) {

ldap/servers/plugins/usn/usn_cleanup.c

@@ -145,6 +145,7 @@ usn_cleanup_thread(void *arg)
     for (ep = entries; ep && *ep; ep++) {
         int delrv = 0;
         const Slapi_DN *sdn = slapi_entry_get_sdn_const(*ep);
+        int opflags = OP_FLAG_TOMBSTONE_ENTRY;
 
         /* check for shutdown */
         if(g_get_shutdown()){
@@ -156,7 +157,7 @@ usn_cleanup_thread(void *arg)
         }
 
         slapi_delete_internal_set_pb(delete_pb, slapi_sdn_get_dn(sdn),
-                                     NULL, NULL, usn_get_identity(), 0);
+                                     NULL, NULL, usn_get_identity(), opflags);
         slapi_delete_internal_pb(delete_pb);
         slapi_pblock_get(delete_pb, SLAPI_PLUGIN_INTOP_RESULT, &delrv);
         if (LDAP_SUCCESS != delrv) {

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

@@ -267,12 +267,8 @@ ldbm_back_delete( Slapi_PBlock *pb )
 	opcsn = operation_get_csn (operation);
 	if (!delete_tombstone_entry)
 	{
-		/* If both USN and replication is enabled, csn set by replication 
-		 * should be honored. */
-		if ((opcsn == NULL || ldbm_usn_enabled(be)) &&
-						!is_fixup_operation && operation->o_csngen_handler)
+		if ((opcsn == NULL) && !is_fixup_operation && operation->o_csngen_handler)
 		{
-			csn_free(&opcsn); /* free opcsn set by USN plugin, if any */
 			/*
 			 * Current op is a user request. Opcsn will be assigned
 			 * by entry_assign_operation_csn() if the dn is in an
@@ -286,14 +282,15 @@ ldbm_back_delete( Slapi_PBlock *pb )
 			{
 				entry_set_maxcsn (e->ep_entry, opcsn);
 			}
-			/*
-			 * We are dealing with replication and if we haven't been called to
-			 * remove a tombstone, then it's because  we want to create a new one.
-			 */
-			if ( slapi_operation_get_replica_attr (pb, operation, "nsds5ReplicaTombstonePurgeInterval", &create_tombstone_entry) == 0)
-			{
-				create_tombstone_entry = (create_tombstone_entry < 0) ? 0 : 1;
-			}
+		}
+		/*
+		 * We are dealing with replication and if we haven't been called to
+		 * remove a tombstone, then it's because  we want to create a new one.
+		 */
+		if ( slapi_operation_get_replica_attr (pb, operation, "nsds5ReplicaTombstonePurgeInterval",
+											   &create_tombstone_entry) == 0 )
+		{
+			create_tombstone_entry = (create_tombstone_entry < 0) ? 0 : 1;
 		}
 	}