Blame SOURCES/0067-Ticket-49551-v3-correct-handling-of-numsubordinates-.patch

081b2d
From 233b64f26df76aa50f4b37aaf6b3804d208fdc1b Mon Sep 17 00:00:00 2001
081b2d
From: Ludwig Krispenz <lkrispen@redhat.com>
081b2d
Date: Mon, 12 Feb 2018 09:24:25 +0100
081b2d
Subject: [PATCH] Ticket 49551 - v3 - correct handling of numsubordinates for
081b2d
 cenotaphs and tombstone delete
081b2d
081b2d
        Bug: The ticket exposed several problems with tombstone handling.
081b2d
                - tombstone entries of conflicts were not purged in tombstone purging
081b2d
                - cenotaphs are tombstone, but the subordinate count was not managed properly
081b2d
                - direct delete of tombstones failed with err=1
081b2d
                - delete of entry with only conflict children failed correctly, but gave no hint why
081b2d
081b2d
        Fix: update the correct numsobordinates attribut for cenotaphs
081b2d
             set proper flag in directly deleting a tombstone
081b2d
             change search filter for tombstone purging to include ldapsubentries
081b2d
             check for conflict children if a delete is rejected and add a message to the response
081b2d
081b2d
        Reviewed by; Thierry, William - thanks
081b2d
---
081b2d
 ldap/servers/plugins/replication/repl5_replica.c | 14 +++++++++--
081b2d
 ldap/servers/plugins/replication/urp.c           |  8 +++---
081b2d
 ldap/servers/slapd/back-ldbm/ldbm_add.c          |  8 +++---
081b2d
 ldap/servers/slapd/back-ldbm/ldbm_delete.c       | 14 ++++++++---
081b2d
 ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c     |  3 ++-
081b2d
 ldap/servers/slapd/back-ldbm/parents.c           | 12 ++++++---
081b2d
 ldap/servers/slapd/entry.c                       | 31 ++++++++++++++++++++++++
081b2d
 ldap/servers/slapd/slapi-plugin.h                |  2 ++
081b2d
 ldap/servers/slapd/slapi-private.h               |  1 +
081b2d
 ldap/servers/slapd/task.c                        |  4 +--
081b2d
 10 files changed, 78 insertions(+), 19 deletions(-)
081b2d
081b2d
diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c
081b2d
index bdb8a5167..628fb9ceb 100644
081b2d
--- a/ldap/servers/plugins/replication/repl5_replica.c
081b2d
+++ b/ldap/servers/plugins/replication/repl5_replica.c
081b2d
@@ -3017,6 +3017,16 @@ process_reap_entry(Slapi_Entry *entry, void *cb_data)
081b2d
        search in the future, see _replica_reap_tombstones below and add more to the
081b2d
        attrs array */
081b2d
     deletion_csn = entry_get_deletion_csn(entry);
081b2d
+    if (deletion_csn == NULL) {
081b2d
+        /* this might be a tombstone which was directly added, eg a cenotaph
081b2d
+         * check if a tombstonecsn exist and use it
081b2d
+         */
081b2d
+        char *tombstonecsn = slapi_entry_attr_get_charptr(entry, SLAPI_ATTR_TOMBSTONE_CSN);
081b2d
+        if (tombstonecsn) {
081b2d
+            deletion_csn = csn_new_by_string(tombstonecsn);
081b2d
+            slapi_ch_free_string(&tombstonecsn);
081b2d
+        }
081b2d
+    }
081b2d
 
081b2d
     if ((NULL == deletion_csn || csn_compare(deletion_csn, purge_csn) < 0) &&
081b2d
         (!is_ruv_tombstone_entry(entry))) {
081b2d
@@ -3116,11 +3126,11 @@ _replica_reap_tombstones(void *arg)
081b2d
              */
081b2d
             csn_as_string(purge_csn, PR_FALSE, deletion_csn_str);
081b2d
             PR_snprintf(tombstone_filter, 128,
081b2d
-                        "(&(%s<=%s)(objectclass=nsTombstone))", SLAPI_ATTR_TOMBSTONE_CSN,
081b2d
+                        "(&(%s<=%s)(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))", SLAPI_ATTR_TOMBSTONE_CSN,
081b2d
                         csn_as_string(purge_csn, PR_FALSE, deletion_csn_str));
081b2d
         } else {
081b2d
             /* Use the old inefficient filter */
081b2d
-            PR_snprintf(tombstone_filter, 128, "(objectclass=nsTombstone)");
081b2d
+            PR_snprintf(tombstone_filter, 128, "(&(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))");
081b2d
         }
081b2d
 
081b2d
         /* we just need the objectclass - for the deletion csn
081b2d
diff --git a/ldap/servers/plugins/replication/urp.c b/ldap/servers/plugins/replication/urp.c
081b2d
index d4556d7fd..11c5da7cf 100644
081b2d
--- a/ldap/servers/plugins/replication/urp.c
081b2d
+++ b/ldap/servers/plugins/replication/urp.c
081b2d
@@ -911,7 +911,7 @@ urp_fixup_add_cenotaph (Slapi_PBlock *pb, char *sessionid, CSN *opcsn)
081b2d
                                     cenotaph,
081b2d
                                     NULL,
081b2d
                                     repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION),
081b2d
-                                    OP_FLAG_REPL_FIXUP|OP_FLAG_NOOP);
081b2d
+                                    OP_FLAG_REPL_FIXUP|OP_FLAG_NOOP|OP_FLAG_CENOTAPH_ENTRY);
081b2d
     slapi_add_internal_pb(add_pb);
081b2d
     slapi_pblock_get(add_pb, SLAPI_PLUGIN_INTOP_RESULT, &ret;;
081b2d
 
081b2d
@@ -1922,7 +1922,7 @@ done:
081b2d
     newpb = NULL;
081b2d
 
081b2d
     slapi_log_err(SLAPI_LOG_REPL, sessionid,
081b2d
-                  "urp_get_min_naming_conflict_entry - Found %d entries\n", i);
081b2d
+                  "urp_get_min_naming_conflict_entry - Found %d entries\n", min_csn?1:0);
081b2d
 
081b2d
     return min_naming_conflict_entry;
081b2d
 }
081b2d
@@ -2172,8 +2172,8 @@ mod_objectclass_attr(const char *uniqueid, const Slapi_DN *entrysdn, const Slapi
081b2d
     char csnstr[CSN_STRSIZE+1] = {0};
081b2d
 
081b2d
     slapi_mods_init(&smods, 3);
081b2d
-    slapi_mods_add(&smods, LDAP_MOD_ADD, "objectclass", strlen("ldapsubentry"),"ldapsubentry");
081b2d
-    slapi_mods_add(&smods, LDAP_MOD_REPLACE, "conflictcsn", CSN_STRSIZE, csn_as_string(opcsn, PR_FALSE, csnstr));
081b2d
+    slapi_mods_add_string(&smods, LDAP_MOD_ADD, "objectclass", "ldapsubentry");
081b2d
+    slapi_mods_add_string(&smods, LDAP_MOD_REPLACE, "conflictcsn", csn_as_string(opcsn, PR_FALSE, csnstr));
081b2d
     op_result = urp_fixup_modify_entry(uniqueid, entrysdn, opcsn, &smods, 0);
081b2d
     slapi_mods_done(&smods);
081b2d
     if (op_result == LDAP_TYPE_OR_VALUE_EXISTS) {
081b2d
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c
081b2d
index c93d44a65..f0a3262ec 100644
081b2d
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
081b2d
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
081b2d
@@ -81,6 +81,7 @@ ldbm_back_add(Slapi_PBlock *pb)
081b2d
     Slapi_Operation *operation;
081b2d
     int is_replicated_operation = 0;
081b2d
     int is_resurect_operation = 0;
081b2d
+    int is_cenotaph_operation = 0;
081b2d
     int is_tombstone_operation = 0;
081b2d
     int is_fixup_operation = 0;
081b2d
     int is_remove_from_cache = 0;
081b2d
@@ -116,6 +117,7 @@ ldbm_back_add(Slapi_PBlock *pb)
081b2d
     }
081b2d
 
081b2d
     is_resurect_operation = operation_is_flag_set(operation, OP_FLAG_RESURECT_ENTRY);
081b2d
+    is_cenotaph_operation = operation_is_flag_set(operation, OP_FLAG_CENOTAPH_ENTRY);
081b2d
     is_tombstone_operation = operation_is_flag_set(operation, OP_FLAG_TOMBSTONE_ENTRY);
081b2d
     is_fixup_operation = operation_is_flag_set(operation, OP_FLAG_REPL_FIXUP);
081b2d
     is_ruv = operation_is_flag_set(operation, OP_FLAG_REPL_RUV);
081b2d
@@ -846,9 +848,9 @@ ldbm_back_add(Slapi_PBlock *pb)
081b2d
                the in-memory state of the parent to reflect the new child (update
081b2d
                subordinate count specifically */
081b2d
             if (parententry) {
081b2d
-                retval = parent_update_on_childchange(&parent_modify_c,
081b2d
-                                                      is_resurect_operation ? PARENTUPDATE_RESURECT : PARENTUPDATE_ADD,
081b2d
-                                                      NULL);
081b2d
+                int op = is_resurect_operation ? PARENTUPDATE_RESURECT : PARENTUPDATE_ADD;
081b2d
+                if (is_cenotaph_operation ) op |= PARENTUPDATE_CREATE_TOMBSTONE;
081b2d
+                retval = parent_update_on_childchange(&parent_modify_c, op, NULL);
081b2d
                 slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_add",
081b2d
                               "conn=%lu op=%d parent_update_on_childchange: old_entry=0x%p, new_entry=0x%p, rc=%d\n",
081b2d
                               conn_id, op_id, parent_modify_c.old_entry, parent_modify_c.new_entry, retval);
081b2d
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
081b2d
index be0db1bd0..bc0a3654e 100644
081b2d
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
081b2d
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
081b2d
@@ -291,9 +291,16 @@ replace_entry:
081b2d
             retval = slapi_entry_has_children(e->ep_entry);
081b2d
             if (retval && !is_replicated_operation) {
081b2d
                 ldap_result_code= LDAP_NOT_ALLOWED_ON_NONLEAF;
081b2d
-                slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete", 
081b2d
-                              "conn=%lu op=%d Deleting entry %s has %d children.\n", 
081b2d
-                               conn_id, op_id, slapi_entry_get_dn(e->ep_entry), retval);
081b2d
+                if (slapi_entry_has_conflict_children(e->ep_entry, (void *)li->li_identity) > 0) {
081b2d
+                    ldap_result_message = "Entry has replication conflicts as children";
081b2d
+                    slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_delete",
081b2d
+                                  "conn=%lu op=%d Deleting entry %s has replication conflicts as children.\n",
081b2d
+                                   conn_id, op_id, slapi_entry_get_dn(e->ep_entry));
081b2d
+                } else {
081b2d
+                    slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete",
081b2d
+                                  "conn=%lu op=%d Deleting entry %s has %d children.\n",
081b2d
+                                   conn_id, op_id, slapi_entry_get_dn(e->ep_entry), retval);
081b2d
+                }
081b2d
                 retval = -1;
081b2d
                 goto error_return;
081b2d
             }
081b2d
@@ -431,6 +438,7 @@ replace_entry:
081b2d
                     slapi_log_err(SLAPI_LOG_WARNING, "ldbm_back_delete",
081b2d
                             "Attempt to Tombstone again a tombstone entry %s\n", dn);
081b2d
                     delete_tombstone_entry = 1;
081b2d
+                    operation_set_flag(operation, OP_FLAG_TOMBSTONE_ENTRY);
081b2d
                 }
081b2d
             }
081b2d
 
081b2d
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
081b2d
index b41a2d241..5797dd779 100644
081b2d
--- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
081b2d
+++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
081b2d
@@ -2824,7 +2824,8 @@ _entryrdn_delete_key(backend *be,
081b2d
                     break;
081b2d
                 }
081b2d
                 childelem = (rdn_elem *)dataret.data;
081b2d
-                if (!slapi_is_special_rdn(childelem->rdn_elem_nrdn_rdn, RDN_IS_TOMBSTONE)) {
081b2d
+                if (!slapi_is_special_rdn(childelem->rdn_elem_nrdn_rdn, RDN_IS_TOMBSTONE) &&
081b2d
+                    !strcasestr(childelem->rdn_elem_nrdn_rdn, "cenotaphid")) {
081b2d
                     /* there's at least one live child */
081b2d
                     slapi_log_err(SLAPI_LOG_ERR, "_entryrdn_delete_key",
081b2d
                                   "Failed to remove %s; has a child %s\n", nrdn,
081b2d
diff --git a/ldap/servers/slapd/back-ldbm/parents.c b/ldap/servers/slapd/back-ldbm/parents.c
081b2d
index 79e66451e..1afc795c0 100644
081b2d
--- a/ldap/servers/slapd/back-ldbm/parents.c
081b2d
+++ b/ldap/servers/slapd/back-ldbm/parents.c
081b2d
@@ -89,7 +89,11 @@ parent_update_on_childchange(modify_context *mc, int op, size_t *new_sub_count)
081b2d
         }
081b2d
     }
081b2d
 
081b2d
-    if (PARENTUPDATE_DELETE_TOMBSTONE != repl_op) {
081b2d
+    if ((PARENTUPDATE_ADD == op) && (PARENTUPDATE_CREATE_TOMBSTONE == repl_op)) {
081b2d
+        /* we are directly adding a tombstone entry, only need to
081b2d
+         * update the tombstone subordinates
081b2d
+         */
081b2d
+    } else if (PARENTUPDATE_DELETE_TOMBSTONE != repl_op) {
081b2d
         /* are we adding ? */
081b2d
         if (((PARENTUPDATE_ADD == op) || (PARENTUPDATE_RESURECT == op)) && !already_present) {
081b2d
             /* If so, and the parent entry does not already have a subcount
081b2d
@@ -136,10 +140,10 @@ parent_update_on_childchange(modify_context *mc, int op, size_t *new_sub_count)
081b2d
         }
081b2d
     }
081b2d
 
081b2d
-    /* tombstoneNumSubordinates is needed only when this is repl op
081b2d
-     * and a child is being deleted */
081b2d
+    /* tombstoneNumSubordinates has to be updated if a tombstone child has been
081b2d
+     * deleted or a tombstone has been directly added (cenotaph) */
081b2d
     current_sub_count = LDAP_MAXINT;
081b2d
-    if ((repl_op && (PARENTUPDATE_DEL == op)) || (PARENTUPDATE_RESURECT == op)) {
081b2d
+    if (repl_op) {
081b2d
         ret = slapi_entry_attr_find(mc->old_entry->ep_entry,
081b2d
                                     tombstone_numsubordinates, &read_attr);
081b2d
         if (0 == ret) {
081b2d
diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c
081b2d
index 32828b4e2..b85e9f5b0 100644
081b2d
--- a/ldap/servers/slapd/entry.c
081b2d
+++ b/ldap/servers/slapd/entry.c
081b2d
@@ -3238,6 +3238,37 @@ slapi_entry_has_children(const Slapi_Entry *entry)
081b2d
     return slapi_entry_has_children_ext(entry, 0);
081b2d
 }
081b2d
 
081b2d
+int
081b2d
+slapi_entry_has_conflict_children(const Slapi_Entry *entry, void *plg_id)
081b2d
+{
081b2d
+    Slapi_PBlock *search_pb = NULL;
081b2d
+    Slapi_Entry **entries;
081b2d
+    int rc = 0;
081b2d
+
081b2d
+    search_pb = slapi_pblock_new();
081b2d
+    slapi_search_internal_set_pb(search_pb, slapi_entry_get_dn_const(entry),
081b2d
+                                 LDAP_SCOPE_ONELEVEL,
081b2d
+                                 "(&(objectclass=ldapsubentry)(nsds5ReplConflict=namingConflict*))",
081b2d
+                                 NULL, 0, NULL, NULL, plg_id, 0);
081b2d
+    slapi_search_internal_pb(search_pb);
081b2d
+    slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &rc);
081b2d
+    if (rc) {
081b2d
+        rc = -1;
081b2d
+    } else {
081b2d
+        slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, &entries);
081b2d
+        if (entries && entries[0]) {
081b2d
+            /* we found at least one conflict entry */
081b2d
+            rc = 1;
081b2d
+        } else {
081b2d
+            rc = 0;
081b2d
+        }
081b2d
+        slapi_free_search_results_internal(search_pb);
081b2d
+    }
081b2d
+    slapi_pblock_destroy(search_pb);
081b2d
+
081b2d
+    return rc;
081b2d
+}
081b2d
+
081b2d
 /*
081b2d
  * Renames an entry to simulate a MODRDN operation
081b2d
  */
081b2d
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
081b2d
index 95cdcc0da..6978e258f 100644
081b2d
--- a/ldap/servers/slapd/slapi-plugin.h
081b2d
+++ b/ldap/servers/slapd/slapi-plugin.h
081b2d
@@ -2000,6 +2000,8 @@ int slapi_entry_has_children(const Slapi_Entry *e);
081b2d
  */
081b2d
 int slapi_entry_has_children_ext(const Slapi_Entry *e, int include_tombstone);
081b2d
 
081b2d
+int slapi_entry_has_conflict_children(const Slapi_Entry *e, void *plg_id);
081b2d
+
081b2d
 /**
081b2d
  * This function determines if an entry is the root DSE.
081b2d
  *
081b2d
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
081b2d
index 548d5cabb..b08c0d7ce 100644
081b2d
--- a/ldap/servers/slapd/slapi-private.h
081b2d
+++ b/ldap/servers/slapd/slapi-private.h
081b2d
@@ -403,6 +403,7 @@ char *slapi_filter_to_string_internal(const struct slapi_filter *f, char *buf, s
081b2d
 #define OP_FLAG_NEVER_CHAIN SLAPI_OP_FLAG_NEVER_CHAIN         /* 0x000800 */
081b2d
 #define OP_FLAG_TOMBSTONE_ENTRY SLAPI_OP_FLAG_TOMBSTONE_ENTRY /* 0x001000 */
081b2d
 #define OP_FLAG_RESURECT_ENTRY 0x002000
081b2d
+#define OP_FLAG_CENOTAPH_ENTRY 0x004000
081b2d
 #define OP_FLAG_ACTION_NOLOG 0x008000            /* Do not log the entry in \
081b2d
                                                   * audit log or change log  \
081b2d
                                                   */
081b2d
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c
081b2d
index 4bd8895ff..3f9d5d995 100644
081b2d
--- a/ldap/servers/slapd/task.c
081b2d
+++ b/ldap/servers/slapd/task.c
081b2d
@@ -2352,10 +2352,10 @@ task_fixup_tombstone_thread(void *arg)
081b2d
 
081b2d
     if (task_data->stripcsn) {
081b2d
         /* find tombstones with nsTombstoneCSN */
081b2d
-        filter = "(&(nstombstonecsn=*)(objectclass=nsTombstone))";
081b2d
+        filter = "(&(nstombstonecsn=*)(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))";
081b2d
     } else {
081b2d
         /* find tombstones missing nsTombstoneCSN */
081b2d
-        filter = "(&(!(nstombstonecsn=*))(objectclass=nsTombstone))";
081b2d
+        filter = "(&(!(nstombstonecsn=*))(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))";
081b2d
     }
081b2d
 
081b2d
     /* Okay check the specified backends only */
081b2d
-- 
081b2d
2.13.6
081b2d