replmd: Minimize get_parsed_dns_trusted() calls during replication

When a group has 10,000+ links, get_parsed_dns_trusted() can be costly
(simply the talloc calls alone are expensive). Instead of re-generating
the pdn_list for every single link attribute, we can change to only
re-generate it when we really need to.

When we add a new link, it reallocates old_el->values, and so we need to
recreate the pdn_list because all the memory pointers will have changed.
However, in the other cases, where we're simply updating the existing
link value (or ignoring the update, if it's already applied), we can
continue using the same pdn_list (rather than re-parsing it again).

This would generally only save time with a full-sync - it won't really
help with the join case (because every link processed results in a
realloc).

On a DB with 5000 users, this makes a full-sync about ~13% faster.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

1 files changed, 26 insertions, 10 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 280a6550a01..433f8f7d6ca 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7939,6 +7939,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
 					   struct la_entry *la_entry,
 					   struct ldb_request *parent,
 					   struct ldb_message_element *old_el,
+					   struct parsed_dn *pdn_list,
 					   replmd_link_changed *change)
 {
 	struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
@@ -7947,7 +7948,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
 	int ret;
 	struct dsdb_dn *dsdb_dn = NULL;
 	uint64_t seq_num = 0;
-	struct parsed_dn *pdn_list, *pdn, *next;
+	struct parsed_dn *pdn, *next;
 	struct GUID guid = GUID_zero();
 	bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
 	bool ignore_link;
@@ -7969,14 +7970,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	/* parse the existing links */
-	ret = get_parsed_dns_trusted(module, replmd_private, mem_ctx, old_el, &pdn_list,
-				     attr->syntax->ldap_oid, parent);
-
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
 	ret = replmd_check_target_exists(module, dsdb_dn, la_entry, msg->dn,
 					 true, &guid, &ignore_link);
 
@@ -8195,6 +8188,7 @@ static int replmd_process_la_group(struct ldb_module *module,
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	const struct dsdb_attribute *attr = NULL;
 	struct ldb_message_element *old_el = NULL;
+	struct parsed_dn *pdn_list = NULL;
 	replmd_link_changed change_type;
 	uint32_t num_changes = 0;
 	time_t t;
@@ -8261,15 +8255,37 @@ static int replmd_process_la_group(struct ldb_module *module,
 	for (la = DLIST_TAIL(la_group->la_entries); la; la=prev) {
 		prev = DLIST_PREV(la);
 		DLIST_REMOVE(la_group->la_entries, la);
+
+		/* parse the existing links */
+		if (pdn_list == NULL) {
+			ret = get_parsed_dns_trusted(module, replmd_private,
+						     tmp_ctx, old_el,
+						     &pdn_list,
+						     attr->syntax->ldap_oid,
+						     NULL);
+
+			if (ret != LDB_SUCCESS) {
+				return ret;
+			}
+		}
 		ret = replmd_process_linked_attribute(module, tmp_ctx,
 						      replmd_private,
 						      msg, attr, la, NULL,
-						      old_el, &change_type);
+						      old_el, pdn_list,
+						      &change_type);
 		if (ret != LDB_SUCCESS) {
 			replmd_txn_cleanup(replmd_private);
 			return ret;
 		}
 
+		/*
+		 * Adding a link reallocs memory, and so invalidates all the
+		 * pointers in pdn_list. Reparse the PDNs on the next loop
+		 */
+		if (change_type == LINK_CHANGE_ADDED) {
+			TALLOC_FREE(pdn_list);
+		}
+
 		if (change_type != LINK_CHANGE_NONE) {
 			num_changes++;
 		}