[PATCH] Batch together processing link attribute for the same source object
Tim Beale
timbeale at catalyst.net.nz
Mon Oct 29 03:52:18 UTC 2018
The attached patches rework the DRS link processing so that we batch
together links for the same source object for efficiency.
This makes the join/clone DRS processing more efficient. For large
databases, this can save thousands of DB search/modify operations.
For a large MDB database (15K users, 225K links), without these changes,
the MDB transaction buffer would completely fill up and fail the join. I
think this is because the transaction spans the entire replication, and
MDB must have been making a separate copy of the object every time it
modifies it (so for a group with 15K users in it, it presumably ends up
with 15K copies of the same group object).
CI link: https://gitlab.com/catalyst-samba/samba/pipelines/34612669
Review/feedback appreciated. Thanks.
-------------- next part --------------
From 732d5ca558f9d95c0a4c8a25f7ceeb20914bf369 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 24 Oct 2018 11:17:38 +1300
Subject: [PATCH 1/5] replmd: Add more debug for replicating links
During a join of a large DB, processing the linked attributes can take a
long time. The join hangs in 'Committing SAM database' for many minutes
with no indication of whether it's making progress or not.
This patch adds some extra debug to show how far through processing the
linked attributes we are, when there are many thousands of links.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 584fd21..d1ac22a 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -75,6 +75,8 @@ struct replmd_private {
struct ldb_dn *schema_dn;
bool originating_updates;
bool sorted_links;
+ uint32_t total_links;
+ uint32_t num_processed;
};
struct la_entry {
@@ -6454,6 +6456,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
}
DLIST_ADD(replmd_private->la_list, la_entry);
+ replmd_private->total_links++;
}
return ret;
@@ -8017,6 +8020,10 @@ static int replmd_prepare_commit(struct ldb_module *module)
struct la_entry *la, *prev;
int ret;
+ if (replmd_private->la_list != NULL) {
+ DBG_NOTICE("Processing linked attributes\n");
+ }
+
/*
* Walk the list of linked attributes from DRS replication.
*
@@ -8033,6 +8040,12 @@ static int replmd_prepare_commit(struct ldb_module *module)
replmd_txn_cleanup(replmd_private);
return ret;
}
+
+ if ((++replmd_private->num_processed % 8192) == 0) {
+ DBG_NOTICE("Processed %u/%u linked attributes\n",
+ replmd_private->num_processed,
+ replmd_private->total_links);
+ }
}
replmd_txn_cleanup(replmd_private);
--
2.7.4
From 72d61aae6308d9e8f4b8be681367f28fed7d97ec Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 24 Oct 2018 12:30:17 +1300
Subject: [PATCH 2/5] replmd: Group together link attribute processing by
source object
Instead of processing each link attribute one at a time, we want to
group them together by source object. This will mean we only have to
look-up the source object once, and only perform one DB 'modify'
operation. With groups with 1000s of members, this will help improve
performance.
This patch takes the first step of group together the links by
source-object. A new 'la_group' struct is added to help track what links
belong to the same source object. The la_list essentially becomes a
'list of lists' now.
Note that only related links *in the same chunk* are only grouped together.
While it is trivial to groups together links that span different
replication chunks, this would be a fairly insignificant efficiency gain,
but seems to have a fairly detrimental memory overhead, once you get
into groups with 10,000+ members.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 101 ++++++++++++++++++++----
1 file changed, 87 insertions(+), 14 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index d1ac22a..37183ad 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -65,7 +65,7 @@ static const NTTIME DELETED_OBJECT_CONTAINER_CHANGE_TIME = 2650466015990000000UL
struct replmd_private {
TALLOC_CTX *la_ctx;
- struct la_entry *la_list;
+ struct la_group *la_list;
struct nc_entry {
struct nc_entry *prev, *next;
struct ldb_dn *dn;
@@ -79,6 +79,20 @@ struct replmd_private {
uint32_t num_processed;
};
+/*
+ * groups link attributes together by source-object and attribute-ID,
+ * to improve processing efficiency (i.e. for 'member' attribute, which
+ * could have 100s or 1000s of links).
+ * Note this grouping is best effort - the same source object could still
+ * correspond to several la_groups (a lot depends on the order DRS sends
+ * the links in). The groups currently don't span replication chunks (which
+ * caps the size to ~1500 links by default).
+ */
+struct la_group {
+ struct la_group *next, *prev;
+ struct la_entry *la_entries;
+};
+
struct la_entry {
struct la_entry *next, *prev;
struct drsuapi_DsReplicaLinkedAttribute *la;
@@ -6406,6 +6420,21 @@ static int replmd_replicated_apply_search_callback(struct ldb_request *req,
}
/**
+ * Returns true if we can group together processing this link attribute,
+ * i.e. it has the same source-object and attribute ID as other links
+ * already in the group
+ */
+static bool la_entry_matches_group(struct la_entry *la_entry,
+ struct la_group *la_group)
+{
+ struct la_entry *prev = la_group->la_entries;
+
+ return (la_entry->la->attid == prev->la->attid &&
+ GUID_equal(&la_entry->la->identifier->guid,
+ &prev->la->identifier->guid));
+}
+
+/**
* Stores the linked attributes received in the replication chunk - these get
* applied at the end of the transaction. We also check that each linked
* attribute is valid, i.e. source and target objects are known.
@@ -6417,6 +6446,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
struct ldb_module *module = ar->module;
struct replmd_private *replmd_private =
talloc_get_type(ldb_module_get_private(module), struct replmd_private);
+ struct la_group *la_group = NULL;
struct ldb_context *ldb;
ldb = ldb_module_get_ctx(module);
@@ -6455,7 +6485,19 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
break;
}
- DLIST_ADD(replmd_private->la_list, la_entry);
+ /* group the links together by source-object for efficiency */
+ if (la_group == NULL ||
+ !la_entry_matches_group(la_entry, la_group)) {
+
+ la_group = talloc_zero(replmd_private->la_ctx,
+ struct la_group);
+ if (la_group == NULL) {
+ ldb_oom(ldb);
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ DLIST_ADD(replmd_private->la_list, la_group);
+ }
+ DLIST_ADD(la_group->la_entries, la_entry);
replmd_private->total_links++;
}
@@ -8009,6 +8051,37 @@ static int replmd_start_transaction(struct ldb_module *module)
return ldb_next_start_trans(module);
}
+/**
+ * Processes a group of linked attributes that apply to the same source-object
+ * and attribute-ID
+ */
+static int replmd_process_la_group(struct ldb_module *module,
+ struct replmd_private *replmd_private,
+ struct la_group *la_group)
+{
+ struct la_entry *la = NULL;
+ struct la_entry *prev = NULL;
+ int ret;
+
+ for (la = DLIST_TAIL(la_group->la_entries); la; la=prev) {
+ prev = DLIST_PREV(la);
+ DLIST_REMOVE(la_group->la_entries, la);
+ ret = replmd_process_linked_attribute(module, replmd_private,
+ la, NULL);
+ if (ret != LDB_SUCCESS) {
+ replmd_txn_cleanup(replmd_private);
+ return ret;
+ }
+
+ if ((++replmd_private->num_processed % 8192) == 0) {
+ DBG_NOTICE("Processed %u/%u linked attributes\n",
+ replmd_private->num_processed,
+ replmd_private->total_links);
+ }
+ }
+ return LDB_SUCCESS;
+}
+
/*
on prepare commit we loop over our queued la_context structures and
apply each of them
@@ -8017,7 +8090,7 @@ static int replmd_prepare_commit(struct ldb_module *module)
{
struct replmd_private *replmd_private =
talloc_get_type(ldb_module_get_private(module), struct replmd_private);
- struct la_entry *la, *prev;
+ struct la_group *la_group, *prev;
int ret;
if (replmd_private->la_list != NULL) {
@@ -8030,22 +8103,22 @@ static int replmd_prepare_commit(struct ldb_module *module)
* We walk backwards, to do the first entry first, as we
* added the entries with DLIST_ADD() which puts them at the
* start of the list
+ *
+ * Links are grouped together so we process links for the same
+ * source object in one go.
*/
- for (la = DLIST_TAIL(replmd_private->la_list); la; la=prev) {
- prev = DLIST_PREV(la);
- DLIST_REMOVE(replmd_private->la_list, la);
- ret = replmd_process_linked_attribute(module, replmd_private,
- la, NULL);
+ for (la_group = DLIST_TAIL(replmd_private->la_list);
+ la_group != NULL;
+ la_group = prev) {
+
+ prev = DLIST_PREV(la_group);
+ DLIST_REMOVE(replmd_private->la_list, la_group);
+ ret = replmd_process_la_group(module, replmd_private,
+ la_group);
if (ret != LDB_SUCCESS) {
replmd_txn_cleanup(replmd_private);
return ret;
}
-
- if ((++replmd_private->num_processed % 8192) == 0) {
- DBG_NOTICE("Processed %u/%u linked attributes\n",
- replmd_private->num_processed,
- replmd_private->total_links);
- }
}
replmd_txn_cleanup(replmd_private);
--
2.7.4
From d8d7238bd8d88c26c970ec516d258cae18f13b8f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 24 Oct 2018 13:25:50 +1300
Subject: [PATCH 3/5] replmd: Split apart source/target checks for links
We've grouped the linked attributes by source-object. Next, we want to
avoid duplicated processing for the source object, i.e. we only need to
check the source object exists once, not once per link.
Before we can do this, we need to tease apart
replmd_extract_la_entry_details(), which is doing both source and target
object processing. Split out extracting the target DSDB-DN so that it's
done separately.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 80 ++++++++++++++-----------
1 file changed, 46 insertions(+), 34 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 37183ad..9bd0fbf 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7293,26 +7293,23 @@ static int replmd_check_target_exists(struct ldb_module *module,
}
/**
- * Extracts the key details about the source/target object for a
+ * Extracts the key details about the source object for a
* linked-attribute entry.
* This returns the following details:
* @param ret_attr the schema details for the linked attribute
* @param source_msg the search result for the source object
- * @param target_dsdb_dn the unpacked DN info for the target object
*/
-static int replmd_extract_la_entry_details(struct ldb_module *module,
- struct la_entry *la_entry,
- TALLOC_CTX *mem_ctx,
- const struct dsdb_attribute **ret_attr,
- struct ldb_message **source_msg,
- struct dsdb_dn **target_dsdb_dn)
+static int replmd_get_la_entry_source(struct ldb_module *module,
+ struct la_entry *la_entry,
+ TALLOC_CTX *mem_ctx,
+ const struct dsdb_attribute **ret_attr,
+ struct ldb_message **source_msg)
{
struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
struct ldb_context *ldb = ldb_module_get_ctx(module);
const struct dsdb_schema *schema = dsdb_get_schema(ldb, mem_ctx);
int ret;
const struct dsdb_attribute *attr;
- WERROR status;
struct ldb_result *res;
const char *attrs[4];
@@ -7403,17 +7400,6 @@ linked_attributes[0]:
}
*source_msg = res->msgs[0];
-
- /* the value blob for the attribute holds the target object DN */
- status = dsdb_dn_la_from_blob(ldb, attr, schema, mem_ctx, la->value.blob, target_dsdb_dn);
- if (!W_ERROR_IS_OK(status)) {
- ldb_asprintf_errstring(ldb, "Failed to parsed linked attribute blob for %s on %s - %s\n",
- attr->lDAPDisplayName,
- ldb_dn_get_linearized(res->msgs[0]->dn),
- win_errstr(status));
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
*ret_attr = attr;
return LDB_SUCCESS;
@@ -7423,19 +7409,23 @@ linked_attributes[0]:
* Verifies the source and target objects are known for a linked attribute
*/
static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
- struct la_entry *la)
+ struct la_entry *la_entry)
{
int ret = LDB_SUCCESS;
- TALLOC_CTX *tmp_ctx = talloc_new(la);
+ TALLOC_CTX *tmp_ctx = talloc_new(la_entry);
struct ldb_module *module = ar->module;
struct ldb_message *src_msg;
const struct dsdb_attribute *attr;
- struct dsdb_dn *tgt_dsdb_dn;
+ struct dsdb_dn *tgt_dsdb_dn = NULL;
struct GUID guid = GUID_zero();
bool dummy;
+ WERROR status;
+ struct ldb_context *ldb = ldb_module_get_ctx(module);
+ struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
+ const struct dsdb_schema *schema = dsdb_get_schema(ldb, tmp_ctx);
- ret = replmd_extract_la_entry_details(module, la, tmp_ctx, &attr,
- &src_msg, &tgt_dsdb_dn);
+ 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
@@ -7453,15 +7443,26 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
return ret;
}
+ /* the value blob for the attribute holds the target object DN */
+ status = dsdb_dn_la_from_blob(ldb, attr, schema, tmp_ctx,
+ la->value.blob, &tgt_dsdb_dn);
+ if (!W_ERROR_IS_OK(status)) {
+ ldb_asprintf_errstring(ldb, "Failed to parsed linked attribute blob for %s on %s - %s\n",
+ attr->lDAPDisplayName,
+ ldb_dn_get_linearized(src_msg->dn),
+ win_errstr(status));
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+
/*
* We can skip the target object checks if we're only syncing critical
* objects, or we know the target is up-to-date. If either case, we
* still continue even if the target doesn't exist
*/
- if ((la->dsdb_repl_flags & (DSDB_REPL_FLAG_OBJECT_SUBSET |
- DSDB_REPL_FLAG_TARGETS_UPTODATE)) == 0) {
+ if ((la_entry->dsdb_repl_flags & (DSDB_REPL_FLAG_OBJECT_SUBSET |
+ DSDB_REPL_FLAG_TARGETS_UPTODATE)) == 0) {
- ret = replmd_check_target_exists(module, tgt_dsdb_dn, la,
+ ret = replmd_check_target_exists(module, tgt_dsdb_dn, la_entry,
src_msg->dn, false, &guid,
&dummy);
}
@@ -7724,7 +7725,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
const struct dsdb_schema *schema = dsdb_get_schema(ldb, tmp_ctx);
int ret;
const struct dsdb_attribute *attr;
- struct dsdb_dn *dsdb_dn;
+ struct dsdb_dn *dsdb_dn = NULL;
uint64_t seq_num = 0;
struct ldb_message_element *old_el;
time_t t = time(NULL);
@@ -7736,13 +7737,14 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
struct dsdb_dn *old_dsdb_dn = NULL;
struct ldb_val *val_to_update = NULL;
bool add_as_inactive = false;
+ WERROR status;
/*
- * get the attribute being modified, the search result for the source object,
- * and the target object's DN details
+ * get the attribute being modified and the search result for the
+ * source object
*/
- ret = replmd_extract_la_entry_details(module, la_entry, tmp_ctx, &attr,
- &msg, &dsdb_dn);
+ ret = replmd_get_la_entry_source(module, la_entry, tmp_ctx, &attr,
+ &msg);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
@@ -7780,10 +7782,20 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
* This is becaue isDeleted is a Boolean, so FALSE is a
* legitimate value (set by Samba's deletetest.py)
*/
-
ldb_msg_remove_attr(msg, "isDeleted");
ldb_msg_remove_attr(msg, "isRecycled");
+ /* the value blob for the attribute holds the target object DN */
+ status = dsdb_dn_la_from_blob(ldb, attr, schema, tmp_ctx,
+ la->value.blob, &dsdb_dn);
+ if (!W_ERROR_IS_OK(status)) {
+ ldb_asprintf_errstring(ldb, "Failed to parsed linked attribute blob for %s on %s - %s\n",
+ attr->lDAPDisplayName,
+ ldb_dn_get_linearized(msg->dn),
+ win_errstr(status));
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+
old_el = ldb_msg_find_element(msg, attr->lDAPDisplayName);
if (old_el == NULL) {
ret = ldb_msg_add_empty(msg, attr->lDAPDisplayName, LDB_FLAG_MOD_REPLACE, &old_el);
--
2.7.4
From 604b278335445bece9794e40ca0bbe6a34ed6df0 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 24 Oct 2018 13:46:06 +1300
Subject: [PATCH 4/5] replmd: Move talloc context one level up
Eventually we want to combine multiple link attributes, that apply to the
same source object, into a single DB 'modify' operation. This will mean
the memory context needs to hang around until we have performed the DB
operation (instead of allocating a temporary context for each link).
This patch moves the talloc context one level up, so a temp context gets
allocated for each link *group*, instead of for each link *attribute*.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 48 ++++++++-----------------
1 file changed, 15 insertions(+), 33 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 9bd0fbf..5313e48 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7714,6 +7714,7 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
process one linked attribute structure
*/
static int replmd_process_linked_attribute(struct ldb_module *module,
+ TALLOC_CTX *mem_ctx,
struct replmd_private *replmd_private,
struct la_entry *la_entry,
struct ldb_request *parent)
@@ -7721,8 +7722,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
struct ldb_context *ldb = ldb_module_get_ctx(module);
struct ldb_message *msg;
- TALLOC_CTX *tmp_ctx = talloc_new(la_entry);
- const struct dsdb_schema *schema = dsdb_get_schema(ldb, tmp_ctx);
+ const struct dsdb_schema *schema = dsdb_get_schema(ldb, mem_ctx);
int ret;
const struct dsdb_attribute *attr;
struct dsdb_dn *dsdb_dn = NULL;
@@ -7743,11 +7743,10 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
* get the attribute being modified and the search result for the
* source object
*/
- ret = replmd_get_la_entry_source(module, la_entry, tmp_ctx, &attr,
+ ret = replmd_get_la_entry_source(module, la_entry, mem_ctx, &attr,
&msg);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7764,7 +7763,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
replmd_deletion_state(module, msg, &deletion_state, NULL);
if (deletion_state >= OBJECT_RECYCLED) {
- talloc_free(tmp_ctx);
return LDB_SUCCESS;
}
@@ -7786,7 +7784,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
ldb_msg_remove_attr(msg, "isRecycled");
/* the value blob for the attribute holds the target object DN */
- status = dsdb_dn_la_from_blob(ldb, attr, schema, tmp_ctx,
+ status = dsdb_dn_la_from_blob(ldb, attr, schema, mem_ctx,
la->value.blob, &dsdb_dn);
if (!W_ERROR_IS_OK(status)) {
ldb_asprintf_errstring(ldb, "Failed to parsed linked attribute blob for %s on %s - %s\n",
@@ -7801,7 +7799,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
ret = ldb_msg_add_empty(msg, attr->lDAPDisplayName, LDB_FLAG_MOD_REPLACE, &old_el);
if (ret != LDB_SUCCESS) {
ldb_module_oom(module);
- talloc_free(tmp_ctx);
return LDB_ERR_OPERATIONS_ERROR;
}
} else {
@@ -7809,11 +7806,10 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
}
/* parse the existing links */
- ret = get_parsed_dns_trusted(module, replmd_private, tmp_ctx, old_el, &pdn_list,
+ ret = get_parsed_dns_trusted(module, replmd_private, mem_ctx, old_el, &pdn_list,
attr->syntax->ldap_oid, parent);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7821,7 +7817,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
true, &guid, &ignore_link);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7830,7 +7825,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
* OK to ignore the linked attribute
*/
if (ignore_link) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7843,22 +7837,19 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
attr->syntax->ldap_oid,
true);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
if (!replmd_link_update_is_newer(pdn, la)) {
DEBUG(3,("Discarding older DRS linked attribute update to %s on %s from %s\n",
old_el->name, ldb_dn_get_linearized(msg->dn),
- GUID_string(tmp_ctx, &la->meta_data.originating_invocation_id)));
- talloc_free(tmp_ctx);
+ GUID_string(mem_ctx, &la->meta_data.originating_invocation_id)));
return LDB_SUCCESS;
}
/* get a seq_num for this change */
ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &seq_num);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7868,13 +7859,12 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
*/
if (active) {
ret = replmd_check_singleval_la_conflict(module, replmd_private,
- tmp_ctx, msg->dn, la,
+ mem_ctx, msg->dn, la,
dsdb_dn, pdn, pdn_list,
old_el, schema, attr,
seq_num,
&add_as_inactive);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
}
@@ -7890,7 +7880,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
&pdn->guid, false, attr,
parent);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
}
@@ -7909,7 +7898,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
offset = old_el->num_values;
} else {
if (next->dsdb_dn == NULL) {
- ret = really_parse_trusted_dn(tmp_ctx, ldb, next,
+ ret = really_parse_trusted_dn(mem_ctx, ldb, next,
attr->syntax->ldap_oid);
if (ret != LDB_SUCCESS) {
return ret;
@@ -7917,7 +7906,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
}
offset = next - pdn_list;
if (offset > old_el->num_values) {
- talloc_free(tmp_ctx);
return LDB_ERR_OPERATIONS_ERROR;
}
}
@@ -7941,14 +7929,13 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
}
/* set the link attribute's value to the info that was received */
- ret = replmd_set_la_val(tmp_ctx, val_to_update, dsdb_dn, old_dsdb_dn,
+ ret = replmd_set_la_val(mem_ctx, val_to_update, dsdb_dn, old_dsdb_dn,
&la->meta_data.originating_invocation_id,
la->meta_data.originating_usn, seq_num,
la->meta_data.originating_change_time,
la->meta_data.version,
!active);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7961,7 +7948,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
val_to_update);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -7974,7 +7960,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
&guid, true, attr,
parent);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
}
@@ -7983,27 +7968,23 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
has changed */
ret = add_time_element(msg, "whenChanged", t);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
ldb_operr(ldb);
return ret;
}
ret = add_uint64_element(ldb, msg, "uSNChanged", seq_num);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
ldb_operr(ldb);
return ret;
}
old_el = ldb_msg_find_element(msg, attr->lDAPDisplayName);
if (old_el == NULL) {
- talloc_free(tmp_ctx);
return ldb_operr(ldb);
}
ret = dsdb_check_single_valued_link(attr, old_el);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -8014,15 +7995,13 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
ldb_debug(ldb, LDB_DEBUG_WARNING, "Failed to apply linked attribute change '%s'\n%s\n",
ldb_errstring(ldb),
ldb_ldif_message_redacted_string(ldb,
- tmp_ctx,
+ mem_ctx,
LDB_CHANGETYPE_MODIFY,
msg));
- talloc_free(tmp_ctx);
return ret;
}
- talloc_free(tmp_ctx);
-
+ TALLOC_FREE(msg);
return ret;
}
@@ -8074,11 +8053,13 @@ static int replmd_process_la_group(struct ldb_module *module,
struct la_entry *la = NULL;
struct la_entry *prev = NULL;
int ret;
+ TALLOC_CTX *tmp_ctx = talloc_new(la_group);
for (la = DLIST_TAIL(la_group->la_entries); la; la=prev) {
prev = DLIST_PREV(la);
DLIST_REMOVE(la_group->la_entries, la);
- ret = replmd_process_linked_attribute(module, replmd_private,
+ ret = replmd_process_linked_attribute(module, tmp_ctx,
+ replmd_private,
la, NULL);
if (ret != LDB_SUCCESS) {
replmd_txn_cleanup(replmd_private);
@@ -8091,6 +8072,7 @@ static int replmd_process_la_group(struct ldb_module *module,
replmd_private->total_links);
}
}
+ TALLOC_FREE(tmp_ctx);
return LDB_SUCCESS;
}
--
2.7.4
From c6a14350383bf72cd5ebaabcafa0420838d6bbf1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 24 Oct 2018 15:40:52 +1300
Subject: [PATCH 5/5] replmd: Single DB operation per source object during link
processing
Move the source object checks and DB modify operation up a level, so we
only do them once per source object rather than once per link.
This allows LMDB joins to succeed with ~15,000 members in a group.
Previously LMDB would fail with the error:
Failed to apply linked attribute change '(-30792) - MDB_MAP_FULL:
Environment mapsize limit reached at ../lib/ldb/ldb_mdb/ldb_mdb.c:203'
Rewriting the same object ~15000 times seemed to completely fill up
the LMDB 8Gb buffer. Presumably this was because LMDB is 'copy on
write', so it was storing ~15,000 copies of the same object. Strangely,
we don't see this problem writing the backlinks (which this patch won't
have helped with at all, because that's modifying the target object).
Note uSNChanged was only being added to the msg once, so the code has
been modified to replace the usnChanged each time (i.e. remove it and
re-add it).
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 131 +++++++++++++-----------
1 file changed, 71 insertions(+), 60 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 5313e48..73937df 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7716,15 +7716,15 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
static int replmd_process_linked_attribute(struct ldb_module *module,
TALLOC_CTX *mem_ctx,
struct replmd_private *replmd_private,
+ struct ldb_message *msg,
+ const struct dsdb_attribute *attr,
struct la_entry *la_entry,
struct ldb_request *parent)
{
struct drsuapi_DsReplicaLinkedAttribute *la = la_entry->la;
struct ldb_context *ldb = ldb_module_get_ctx(module);
- struct ldb_message *msg;
const struct dsdb_schema *schema = dsdb_get_schema(ldb, mem_ctx);
int ret;
- const struct dsdb_attribute *attr;
struct dsdb_dn *dsdb_dn = NULL;
uint64_t seq_num = 0;
struct ldb_message_element *old_el;
@@ -7733,56 +7733,11 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
struct GUID guid = GUID_zero();
bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
bool ignore_link;
- enum deletion_state deletion_state = OBJECT_NOT_DELETED;
struct dsdb_dn *old_dsdb_dn = NULL;
struct ldb_val *val_to_update = NULL;
bool add_as_inactive = false;
WERROR status;
- /*
- * get the attribute being modified and the search result for the
- * source object
- */
- ret = replmd_get_la_entry_source(module, la_entry, mem_ctx, &attr,
- &msg);
-
- if (ret != LDB_SUCCESS) {
- return ret;
- }
-
- /*
- * Check for deleted objects per MS-DRSR 4.1.10.6.14
- * ProcessLinkValue, because link updates are not applied to
- * recycled and tombstone objects. We don't have to delete
- * any existing link, that should have happened when the
- * object deletion was replicated or initiated.
- *
- * This needs isDeleted and isRecycled to be included as
- * attributes in the search and so in msg if set.
- */
- replmd_deletion_state(module, msg, &deletion_state, NULL);
-
- if (deletion_state >= OBJECT_RECYCLED) {
- return LDB_SUCCESS;
- }
-
- /*
- * Now that we know the deletion_state, remove the extra
- * attributes added for that purpose. We need to do this
- * otherwise in the case of isDeleted: FALSE the modify will
- * fail with:
- *
- * Failed to apply linked attribute change 'attribute 'isDeleted':
- * invalid modify flags on
- * 'CN=g1_1527570609273,CN=Users,DC=samba,DC=example,DC=com':
- * 0x0'
- *
- * This is becaue isDeleted is a Boolean, so FALSE is a
- * legitimate value (set by Samba's deletetest.py)
- */
- ldb_msg_remove_attr(msg, "isDeleted");
- ldb_msg_remove_attr(msg, "isRecycled");
-
/* the value blob for the attribute holds the target object DN */
status = dsdb_dn_la_from_blob(ldb, attr, schema, mem_ctx,
la->value.blob, &dsdb_dn);
@@ -7966,6 +7921,8 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
/* we only change whenChanged and uSNChanged if the seq_num
has changed */
+ ldb_msg_remove_attr(msg, "whenChanged");
+ ldb_msg_remove_attr(msg, "uSNChanged");
ret = add_time_element(msg, "whenChanged", t);
if (ret != LDB_SUCCESS) {
ldb_operr(ldb);
@@ -7990,18 +7947,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
old_el->flags |= LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK;
- ret = linked_attr_modify(module, msg, parent);
- if (ret != LDB_SUCCESS) {
- ldb_debug(ldb, LDB_DEBUG_WARNING, "Failed to apply linked attribute change '%s'\n%s\n",
- ldb_errstring(ldb),
- ldb_ldif_message_redacted_string(ldb,
- mem_ctx,
- LDB_CHANGETYPE_MODIFY,
- msg));
- return ret;
- }
-
- TALLOC_FREE(msg);
return ret;
}
@@ -8054,13 +7999,64 @@ static int replmd_process_la_group(struct ldb_module *module,
struct la_entry *prev = NULL;
int ret;
TALLOC_CTX *tmp_ctx = talloc_new(la_group);
+ struct la_entry *first_la = DLIST_TAIL(la_group->la_entries);
+ struct ldb_message *msg = NULL;
+ enum deletion_state deletion_state = OBJECT_NOT_DELETED;
+ struct ldb_context *ldb = ldb_module_get_ctx(module);
+ const struct dsdb_attribute *attr = NULL;
+
+ /*
+ * get the attribute being modified and the search result for the
+ * source object
+ */
+ ret = replmd_get_la_entry_source(module, first_la, tmp_ctx, &attr,
+ &msg);
+
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ /*
+ * Check for deleted objects per MS-DRSR 4.1.10.6.14
+ * ProcessLinkValue, because link updates are not applied to
+ * recycled and tombstone objects. We don't have to delete
+ * any existing link, that should have happened when the
+ * object deletion was replicated or initiated.
+ *
+ * This needs isDeleted and isRecycled to be included as
+ * attributes in the search and so in msg if set.
+ */
+ replmd_deletion_state(module, msg, &deletion_state, NULL);
+ if (deletion_state >= OBJECT_RECYCLED) {
+ TALLOC_FREE(tmp_ctx);
+ return LDB_SUCCESS;
+ }
+
+ /*
+ * Now that we know the deletion_state, remove the extra
+ * attributes added for that purpose. We need to do this
+ * otherwise in the case of isDeleted: FALSE the modify will
+ * fail with:
+ *
+ * Failed to apply linked attribute change 'attribute 'isDeleted':
+ * invalid modify flags on
+ * 'CN=g1_1527570609273,CN=Users,DC=samba,DC=example,DC=com':
+ * 0x0'
+ *
+ * This is becaue isDeleted is a Boolean, so FALSE is a
+ * legitimate value (set by Samba's deletetest.py)
+ */
+ ldb_msg_remove_attr(msg, "isDeleted");
+ ldb_msg_remove_attr(msg, "isRecycled");
+
+ /* go through and process the link targets for this source object */
for (la = DLIST_TAIL(la_group->la_entries); la; la=prev) {
prev = DLIST_PREV(la);
DLIST_REMOVE(la_group->la_entries, la);
ret = replmd_process_linked_attribute(module, tmp_ctx,
replmd_private,
- la, NULL);
+ msg, attr, la, NULL);
if (ret != LDB_SUCCESS) {
replmd_txn_cleanup(replmd_private);
return ret;
@@ -8072,6 +8068,21 @@ static int replmd_process_la_group(struct ldb_module *module,
replmd_private->total_links);
}
}
+
+ /* apply the link changes to the source object */
+ ret = linked_attr_modify(module, msg, NULL);
+ if (ret != LDB_SUCCESS) {
+ ldb_debug(ldb, LDB_DEBUG_WARNING,
+ "Failed to apply linked attribute change '%s'\n%s\n",
+ ldb_errstring(ldb),
+ ldb_ldif_message_redacted_string(ldb,
+ tmp_ctx,
+ LDB_CHANGETYPE_MODIFY,
+ msg));
+ TALLOC_FREE(tmp_ctx);
+ return ret;
+ }
+
TALLOC_FREE(tmp_ctx);
return LDB_SUCCESS;
}
--
2.7.4
More information about the samba-technical
mailing list