replmd: Skip redundant source object link checks
authorTim Beale <timbeale@catalyst.net.nz>
Fri, 9 Nov 2018 01:06:16 +0000 (14:06 +1300)
committerTim Beale <timbeale@samba.org>
Tue, 20 Nov 2018 03:48:46 +0000 (04:48 +0100)
We receive the links grouped together by source object. We can save
ourselves some work by not looking up the source object for every single
link (if it's still the same object we're dealing with).

We've already made this change to replmd_process_linked_attribute().
This patch makes the same change to replmd_store_linked_attributes().
(We verify that we know about each link source/target as we receive each
replication chunk. replmd_process_linked_attribute() kicks in later as
the transaction completes).

Note some care is needed to hold onto the tmp_ctx/src_msg across
multiple passes of the for loop.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
source4/dsdb/samdb/ldb_modules/repl_meta_data.c

index 5bf819382f225dbd6d94df567922f5773247a8eb..3897e9eafc4b4c0f210edd103988d75ac53c7bd7 100644 (file)
@@ -6592,8 +6592,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
        /* save away the linked attributes for the end of the transaction */
        for (i = 0; i < ar->objs->linked_attributes_count; i++) {
                struct la_entry *la_entry;
-
-               tmp_ctx = talloc_new(ar);
+               bool new_srcobj;
 
                if (replmd_private->la_ctx == NULL) {
                        replmd_private->la_ctx = talloc_new(replmd_private);
@@ -6617,22 +6616,42 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
                talloc_steal(la_entry->la, la_entry->la->identifier);
                talloc_steal(la_entry->la, la_entry->la->value.blob);
 
-               /* verify the source object exists for the link */
-               ret = replmd_get_la_entry_source(module, la_entry, tmp_ctx,
-                                                &attr, &src_msg);
-
                /*
-                * When we fail to find the source object, the error
-                * code we pass back here is really important. It flags
-                * back to the callers to retry this request with
-                * DRSUAPI_DRS_GET_ANC. This case should never happen
-                * if we're replicating from a Samba DC, but it is
-                * needed to talk to a Windows DC
+                * check if we're still dealing with the same source object
+                * as the last link
                 */
-               if (ret == LDB_ERR_NO_SUCH_OBJECT) {
-                       WERROR err = WERR_DS_DRA_MISSING_PARENT;
-                       ret = replmd_replicated_request_werror(ar, err);
-                       break;
+               new_srcobj = (la_group == NULL ||
+                             !la_entry_matches_group(la_entry, la_group));
+
+               if (new_srcobj) {
+
+                       /* get a new mem_ctx to lookup the source object */
+                       TALLOC_FREE(tmp_ctx);
+                       tmp_ctx = talloc_new(ar);
+                       if (tmp_ctx == NULL) {
+                               ldb_oom(ldb);
+                               return LDB_ERR_OPERATIONS_ERROR;
+                       }
+
+                       /* verify the link source exists */
+                       ret = replmd_get_la_entry_source(module, la_entry,
+                                                        tmp_ctx, &attr,
+                                                        &src_msg);
+
+                       /*
+                        * When we fail to find the source object, the error
+                        * code we pass back here is really important. It flags
+                        * back to the callers to retry this request with
+                        * DRSUAPI_DRS_GET_ANC. This case should never happen
+                        * if we're replicating from a Samba DC, but it is
+                        * needed to talk to a Windows DC
+                        */
+                       if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+                               WERROR err = WERR_DS_DRA_MISSING_PARENT;
+                               ret = replmd_replicated_request_werror(ar,
+                                                                      err);
+                               break;
+                       }
                }
 
                ret = replmd_verify_link_target(ar, tmp_ctx, la_entry,
@@ -6642,9 +6661,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
                }
 
                /* group the links together by source-object for efficiency */
-               if (la_group == NULL ||
-                   !la_entry_matches_group(la_entry, la_group)) {
-
+               if (new_srcobj) {
                        la_group = talloc_zero(replmd_private->la_ctx,
                                               struct la_group);
                        if (la_group == NULL) {
@@ -6655,9 +6672,9 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
                }
                DLIST_ADD(la_group->la_entries, la_entry);
                replmd_private->total_links++;
-               TALLOC_FREE(tmp_ctx);
        }
 
+       TALLOC_FREE(tmp_ctx);
        return ret;
 }