[SCM] Samba Shared Repository - branch master updated
Tim Beale
timbeale at samba.org
Tue Nov 20 07:41:02 UTC 2018
The branch, master has been updated
via 8b47443b871 replmd: Cache recycle-bin state to avoid DB lookup
via 062ac12a310 replmd: Split some code out into create_la_entry() helper function
via c49c0c300ce replmd: Minor change to replmd_verify_link_target() args
via ffe7707675f replmd: Skip redundant source object link checks
via af1f24acf78 replmd: Split up replmd_verify_linked_attribute() into src/target checks
from 0025e8e96a7 waf: Load the C compiler correctly
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 8b47443b871c8cfcae60f4d098ff27e561ee6cd4
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri Oct 26 15:04:42 2018 +1300
replmd: Cache recycle-bin state to avoid DB lookup
By caching the recycle-bin state we can save ~6% of the join time.
Checking whether the recycle-bin is enabled involves an underlying DSDB
search. We do this ~4 times for each link we replicate (twice for the
link source and target). By caching the recycle-bin's state over the
duration of the replication, we can save 1000s of unnecessary DB
searches.
With 5K users this makes the join time ~5 secs faster.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Tim Beale <timbeale at samba.org>
Autobuild-Date(master): Tue Nov 20 08:40:16 CET 2018 on sn-devel-144
commit 062ac12a31010436953ebe0b31351f7ec0844415
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri Nov 9 14:29:14 2018 +1300
replmd: Split some code out into create_la_entry() helper function
replmd_store_linked_attributes() has gotten in szie and complexity. This
refactors some code out into a separate function to make things a bit
more manageable.
This patch should not alter functionality.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit c49c0c300ce2d4b9f11039640bfd58265c7477af
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri Nov 9 14:13:11 2018 +1300
replmd: Minor change to replmd_verify_link_target() args
We were passing in the entire src_msg, but all we really need is the
source object's DN (and even then, it's only used in error messages).
Change it so we only pass in what the function actually needs. This
makes it a bit easier to see what src_msg is actually used for.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ffe7707675fe91830bea59725e324d029cf2988d
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri Nov 9 14:06:16 2018 +1300
replmd: Skip redundant source object link checks
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 at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit af1f24acf783c293d3af6de8e10a64a036d7be38
Author: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri Oct 26 15:43:33 2018 +1300
replmd: Split up replmd_verify_linked_attribute() into src/target checks
Refactor replmd_verify_linked_attribute() so we split out the link
attribute source/target checks. This patch should not alter
functionality.
The source object check has been moved out to where
replmd_verify_linked_attribute() was called.
replmd_verify_linked_attribute() has been renamed, as it's now only
checking the link target.
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 | 192 ++++++++++++++++--------
1 file changed, 133 insertions(+), 59 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 3c7b8b16465..58f9df9c98c 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -77,6 +77,8 @@ struct replmd_private {
bool sorted_links;
uint32_t total_links;
uint32_t num_processed;
+ bool recyclebin_enabled;
+ bool recyclebin_state_known;
};
/*
@@ -138,8 +140,16 @@ static int replmd_check_upgrade_links(struct ldb_context *ldb,
struct parsed_dn *dns, uint32_t count,
struct ldb_message_element *el,
const char *ldap_oid);
-static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
- struct la_entry *la);
+static int replmd_verify_link_target(struct replmd_replicated_request *ar,
+ TALLOC_CTX *mem_ctx,
+ struct la_entry *la_entry,
+ struct ldb_dn *src_dn,
+ const struct dsdb_attribute *attr);
+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);
static int replmd_set_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
struct dsdb_dn *old_dsdb_dn, const struct GUID *invocation_id,
uint64_t usn, uint64_t local_usn, NTTIME nttime,
@@ -166,12 +176,35 @@ enum deletion_state {
OBJECT_REMOVED=5
};
+static bool replmd_recyclebin_enabled(struct ldb_module *module)
+{
+ bool enabled = false;
+ struct replmd_private *replmd_private =
+ talloc_get_type_abort(ldb_module_get_private(module),
+ struct replmd_private);
+
+ /*
+ * only lookup the recycle-bin state once per replication, then cache
+ * the result. This can save us 1000s of DB searches
+ */
+ if (!replmd_private->recyclebin_state_known) {
+ int ret = dsdb_recyclebin_enabled(module, &enabled);
+ if (ret != LDB_SUCCESS) {
+ return false;
+ }
+
+ replmd_private->recyclebin_enabled = enabled;
+ replmd_private->recyclebin_state_known = true;
+ }
+
+ return replmd_private->recyclebin_enabled;
+}
+
static void replmd_deletion_state(struct ldb_module *module,
const struct ldb_message *msg,
enum deletion_state *current_state,
enum deletion_state *next_state)
{
- int ret;
bool enabled = false;
if (msg == NULL) {
@@ -182,10 +215,7 @@ static void replmd_deletion_state(struct ldb_module *module,
return;
}
- ret = dsdb_recyclebin_enabled(module, &enabled);
- if (ret != LDB_SUCCESS) {
- enabled = false;
- }
+ enabled = replmd_recyclebin_enabled(module);
if (ldb_msg_check_string_attribute(msg, "isDeleted", "TRUE")) {
if (!enabled) {
@@ -326,7 +356,7 @@ static void replmd_txn_cleanup(struct replmd_private *replmd_private)
talloc_free(replmd_private->la_ctx);
replmd_private->la_list = NULL;
replmd_private->la_ctx = NULL;
-
+ replmd_private->recyclebin_state_known = false;
}
@@ -6559,6 +6589,43 @@ static bool la_entry_matches_group(struct la_entry *la_entry,
&prev->la->identifier->guid));
}
+/**
+ * Creates a new la_entry to store replication info for a single
+ * linked attribute.
+ */
+static struct la_entry *
+create_la_entry(struct replmd_private *replmd_private,
+ struct drsuapi_DsReplicaLinkedAttribute *la,
+ uint32_t dsdb_repl_flags)
+{
+ struct la_entry *la_entry;
+
+ if (replmd_private->la_ctx == NULL) {
+ replmd_private->la_ctx = talloc_new(replmd_private);
+ }
+ la_entry = talloc(replmd_private->la_ctx, struct la_entry);
+ if (la_entry == NULL) {
+ return NULL;
+ }
+ la_entry->la = talloc(la_entry,
+ struct drsuapi_DsReplicaLinkedAttribute);
+ if (la_entry->la == NULL) {
+ talloc_free(la_entry);
+ return NULL;
+ }
+ *la_entry->la = *la;
+ la_entry->dsdb_repl_flags = dsdb_repl_flags;
+
+ /*
+ * we need to steal the non-scalars so they stay
+ * around until the end of the transaction
+ */
+ talloc_steal(la_entry->la, la_entry->la->identifier);
+ talloc_steal(la_entry->la, la_entry->la->value.blob);
+
+ return la_entry;
+}
+
/**
* Stores the linked attributes received in the replication chunk - these get
* applied at the end of the transaction. We also check that each linked
@@ -6573,6 +6640,9 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
talloc_get_type(ldb_module_get_private(module), struct replmd_private);
struct la_group *la_group = NULL;
struct ldb_context *ldb;
+ TALLOC_CTX *tmp_ctx = NULL;
+ struct ldb_message *src_msg = NULL;
+ const struct dsdb_attribute *attr = NULL;
ldb = ldb_module_get_ctx(module);
@@ -6581,39 +6651,63 @@ 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;
+ bool new_srcobj;
- if (replmd_private->la_ctx == NULL) {
- replmd_private->la_ctx = talloc_new(replmd_private);
- }
- la_entry = talloc(replmd_private->la_ctx, struct la_entry);
+ /* create an entry to store the received link attribute info */
+ la_entry = create_la_entry(replmd_private,
+ &ar->objs->linked_attributes[i],
+ ar->objs->dsdb_repl_flags);
if (la_entry == NULL) {
ldb_oom(ldb);
return LDB_ERR_OPERATIONS_ERROR;
}
- la_entry->la = talloc(la_entry, struct drsuapi_DsReplicaLinkedAttribute);
- if (la_entry->la == NULL) {
- talloc_free(la_entry);
- ldb_oom(ldb);
- return LDB_ERR_OPERATIONS_ERROR;
- }
- *la_entry->la = ar->objs->linked_attributes[i];
- la_entry->dsdb_repl_flags = ar->objs->dsdb_repl_flags;
- /* we need to steal the non-scalars so they stay
- around until the end of the transaction */
- talloc_steal(la_entry->la, la_entry->la->identifier);
- talloc_steal(la_entry->la, la_entry->la->value.blob);
+ /*
+ * check if we're still dealing with the same source object
+ * as the last link
+ */
+ new_srcobj = (la_group == NULL ||
+ !la_entry_matches_group(la_entry, la_group));
+
+ if (new_srcobj) {
- ret = replmd_verify_linked_attribute(ar, la_entry);
+ /* 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,
+ src_msg->dn, attr);
if (ret != LDB_SUCCESS) {
break;
}
/* 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) {
@@ -6626,6 +6720,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
replmd_private->total_links++;
}
+ TALLOC_FREE(tmp_ctx);
return ret;
}
@@ -7531,50 +7626,31 @@ linked_attributes[0]:
}
/**
- * Verifies the source and target objects are known for a linked attribute
+ * Verifies the target object is known for a linked attribute
*/
-static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
- struct la_entry *la_entry)
+static int replmd_verify_link_target(struct replmd_replicated_request *ar,
+ TALLOC_CTX *mem_ctx,
+ struct la_entry *la_entry,
+ struct ldb_dn *src_dn,
+ const struct dsdb_attribute *attr)
{
int ret = LDB_SUCCESS;
- 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 = 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_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) {
- ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_MISSING_PARENT);
- }
-
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
+ const struct dsdb_schema *schema = dsdb_get_schema(ldb, mem_ctx);
/* 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, &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),
+ ldb_dn_get_linearized(src_dn),
win_errstr(status));
return LDB_ERR_OPERATIONS_ERROR;
}
@@ -7588,8 +7664,7 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
DSDB_REPL_FLAG_TARGETS_UPTODATE)) == 0) {
ret = replmd_check_target_exists(module, tgt_dsdb_dn, la_entry,
- src_msg->dn, false, &guid,
- &dummy);
+ src_dn, false, &guid, &dummy);
}
/*
@@ -7601,7 +7676,6 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
ret = replmd_replicated_request_werror(ar, WERR_DS_DRA_RECYCLED_TARGET);
}
- talloc_free(tmp_ctx);
return ret;
}
--
Samba Shared Repository
More information about the samba-cvs
mailing list