From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 10 Feb 2022 11:46:10 +0000 (+0100)
Subject: s4:dsdb/descriptor: skip duplicates in descriptor_sd_propagation_object()
X-Git-Tag: tevent-0.12.0~201
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f7f65ceb46d04e48667e6cba8f3e9b9fd0cd290e;p=thirdparty%2Fsamba.git

s4:dsdb/descriptor: skip duplicates in descriptor_sd_propagation_object()

We're now sure that the security descriptor propagation happened
first for parent objects.

It means we can safely skip processing the same object twice in
descriptor_sd_propagation_object().

For the database with ~ 22000 objects it reduced the commit time
from 2m 50s down to 2m 24s.

The statistics are changed from:

descriptor_prepare_commit: changes: num_registrations=50000
descriptor_prepare_commit: changes: num_registered=22000
descriptor_prepare_commit: changes: num_toplevel=5
descriptor_prepare_commit: changes: num_processed=5200
descriptor_prepare_commit: objects: num_processed=68800

to:

descriptor_prepare_commit: changes: num_registrations=50000
descriptor_prepare_commit: changes: num_registered=22000
descriptor_prepare_commit: changes: num_toplevel=5
descriptor_prepare_commit: changes: num_processed=5200
descriptor_prepare_commit: objects: num_processed=22000
descriptor_prepare_commit: objects: num_skipped=41600

It means that we have "changes: num_registered" and
"objects: num_processed" exactly match the number
of replicated objects.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Wed Mar 30 12:06:21 UTC 2022 on sn-devel-184
---

diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
index 6e96361117a..0f221b794d4 100644
--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
@@ -90,7 +90,9 @@ struct descriptor_transaction {
 		size_t num_processed;
 	} changes;
 	struct {
+		struct db_context *map;
 		size_t num_processed;
+		size_t num_skipped;
 	} objects;
 };
 
@@ -1088,6 +1090,11 @@ static void descriptor_changes_parser(TDB_DATA key, TDB_DATA data, void *private
 	*c_ptr = talloc_get_type_abort((void *)ptr, struct descriptor_changes);
 }
 
+static void descriptor_object_parser(TDB_DATA key, TDB_DATA data, void *private_data)
+{
+	SMB_ASSERT(data.dsize == 0);
+}
+
 static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
 						    struct ldb_request *req)
 {
@@ -1263,25 +1270,63 @@ static int descriptor_sd_propagation_object(struct ldb_module *module,
 	struct GUID guid;
 	int ret;
 	TDB_DATA key;
+	TDB_DATA empty_val = { .dsize = 0, };
 	NTSTATUS status;
 	struct descriptor_changes *c = NULL;
 
 	*stop = false;
 
-	t->objects.num_processed += 1;
-
 	/*
 	 * We get the GUID of the object
-	 * in order to check if there's
-	 * a descriptor_changes in our list.
+	 * in order to have the cache key
+	 * for the object.
 	 */
 
 	status = dsdb_get_extended_dn_guid(msg->dn, &guid, "GUID");
 	if (!NT_STATUS_IS_OK(status)) {
 		return ldb_operr(ldb);
 	}
-
 	key = make_tdb_data((const void*)&guid, sizeof(guid));
+
+	/*
+	 * Check if we already processed this object.
+	 */
+	status = dbwrap_parse_record(t->objects.map, key,
+				     descriptor_object_parser, NULL);
+	if (NT_STATUS_IS_OK(status)) {
+		/*
+		 * All work is already one
+		 */
+		t->objects.num_skipped += 1;
+		*stop = true;
+		return LDB_SUCCESS;
+	}
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+		ldb_debug(ldb, LDB_DEBUG_FATAL,
+			  "dbwrap_parse_record() - %s\n",
+			  nt_errstr(status));
+		return ldb_module_operr(module);
+	}
+
+	t->objects.num_processed += 1;
+
+	/*
+	 * Remember that we're processing this object.
+	 */
+	status = dbwrap_store(t->objects.map, key, empty_val, TDB_INSERT);
+	if (!NT_STATUS_IS_OK(status)) {
+		ldb_debug(ldb, LDB_DEBUG_FATAL,
+			  "dbwrap_parse_record() - %s\n",
+			  nt_errstr(status));
+		return ldb_module_operr(module);
+	}
+
+	/*
+	 * Check that if there's a descriptor_change in our list,
+	 * which we may be able to remove from the pending list
+	 * when we processed the object.
+	 */
+
 	status = dbwrap_parse_record(t->changes.map, key, descriptor_changes_parser, &c);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
 		c = NULL;
@@ -1599,6 +1644,12 @@ static int descriptor_start_transaction(struct ldb_module *module)
 		*t = (struct descriptor_transaction) { .mem = NULL, };
 		return ldb_module_oom(module);
 	}
+	t->objects.map = db_open_rbt(t->mem);
+	if (t->objects.map == NULL) {
+		TALLOC_FREE(t->mem);
+		*t = (struct descriptor_transaction) { .mem = NULL, };
+		return ldb_module_oom(module);
+	}
 
 	return ldb_next_start_trans(module);
 }
@@ -1748,6 +1799,7 @@ static int descriptor_prepare_commit(struct ldb_module *module)
 
 	DBG_NOTICE("changes: num_processed=%zu\n", t->changes.num_processed);
 	DBG_NOTICE("objects: num_processed=%zu\n", t->objects.num_processed);
+	DBG_NOTICE("objects: num_skipped=%zu\n", t->objects.num_skipped);
 
 	return ldb_next_prepare_commit(module);
 }