[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Thu Nov 1 22:49:02 UTC 2018
The branch, master has been updated
via 775054a replmd: Single DB operation per source object during link processing
via d54956f replmd: Move talloc context one level up
via 0ba0286 replmd: Split apart source/target checks for links
via 628b9f7 replmd: Group together link attribute processing by source object
via 5528a4c replmd: Add more debug for replicating links
from 9b28d47 torture: Fix the clang build
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 775054afbe15129691c76f3b276ab4473cbc995b
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed Oct 24 15:40:52 2018 +1300
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Thu Nov 1 23:48:21 CET 2018 on sn-devel-144
commit d54956fd51f356439173b24833ee88b38719fb80
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed Oct 24 13:46:06 2018 +1300
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 0ba028681dc337acf9c8c7f71fa33a8a09b87c58
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed Oct 24 13:25:50 2018 +1300
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 628b9f71c7d5cf2b76cd9f6a87bc30ac8fa14167
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed Oct 24 12:30:17 2018 +1300
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 5528a4cce4f7520bec3305f5ba2951378184687d
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed Oct 24 11:17:38 2018 +1300
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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 337 +++++++++++++++---------
1 file changed, 214 insertions(+), 123 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index b1f1523..3c7b8b1 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;
@@ -75,6 +75,22 @@ struct replmd_private {
struct ldb_dn *schema_dn;
bool originating_updates;
bool sorted_links;
+ uint32_t total_links;
+ 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 {
@@ -6529,6 +6545,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.
@@ -6540,6 +6571,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);
@@ -6578,7 +6610,20 @@ 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++;
}
return ret;
@@ -7373,26 +7418,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];
@@ -7483,17 +7525,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;
@@ -7503,19 +7534,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
@@ -7533,15 +7568,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);
}
@@ -7801,18 +7847,18 @@ 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 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;
- 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;
+ struct dsdb_dn *dsdb_dn = NULL;
uint64_t seq_num = 0;
struct ldb_message_element *old_el;
time_t t = time(NULL);
@@ -7820,64 +7866,27 @@ 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, the search result for the source object,
- * and the target object's DN details
- */
- ret = replmd_extract_la_entry_details(module, la_entry, tmp_ctx, &attr,
- &msg, &dsdb_dn);
-
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- 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;
+ /* 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);
+ 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;
}
- /*
- * 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");
-
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);
if (ret != LDB_SUCCESS) {
ldb_module_oom(module);
- talloc_free(tmp_ctx);
return LDB_ERR_OPERATIONS_ERROR;
}
} else {
@@ -7885,11 +7894,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;
}
@@ -7897,7 +7905,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;
}
@@ -7906,7 +7913,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;
}
@@ -7919,22 +7925,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;
}
@@ -7944,13 +7947,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;
}
}
@@ -7966,7 +7968,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;
}
}
@@ -7985,7 +7986,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;
@@ -7993,7 +7994,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;
}
}
@@ -8017,14 +8017,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;
}
@@ -8037,7 +8036,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
val_to_update);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
@@ -8050,55 +8048,38 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
&guid, true, attr,
parent);
if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
return ret;
}
}
/* 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) {
- 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;
}
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,
- tmp_ctx,
- LDB_CHANGETYPE_MODIFY,
- msg));
- talloc_free(tmp_ctx);
- return ret;
- }
-
- talloc_free(tmp_ctx);
-
return ret;
}
@@ -8139,6 +8120,106 @@ 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;
+ 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);
--
Samba Shared Repository
More information about the samba-cvs
mailing list