From c3dc979e962796dbbfba7dbc3ae49caacf2f2ee4 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: May 24 2012 20:53:19 +0000 Subject: 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) --- diff --git a/ldap/servers/plugins/usn/usn.c b/ldap/servers/plugins/usn/usn.c index 812d83d..0273fd1 100644 --- a/ldap/servers/plugins/usn/usn.c +++ b/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) { diff --git a/ldap/servers/plugins/usn/usn_cleanup.c b/ldap/servers/plugins/usn/usn_cleanup.c index 20decae..6f9410e 100644 --- a/ldap/servers/plugins/usn/usn_cleanup.c +++ b/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) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 846d87b..0427b45 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/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; } }