ldb one-level search performance issue
Andrew Bartlett
abartlet at samba.org
Mon May 28 01:23:18 UTC 2018
On Sat, 2018-05-26 at 15:43 +1200, Andrew Bartlett via samba-technical
wrote:
> On Sat, 2018-05-26 at 07:20 +1200, Andrew Bartlett via samba-technical
> wrote:
> >
> > The string component stuff was a red herring (as it would not throw an
> > error), but it hid this real issue:
> >
> > Checking 18862 objects
> > ERROR: parent object not found for CN=kcc_BCDE3
> >
> > Patches to get that far are in
> >
> > https://gitlab.com/catalyst-samba/samba/commits/abartlet-dbcheck-ideas
>
> With that branch, the real issue can be reproduced in just two tests
> with:
>
> make test TESTS="kcc dbcheck --include-env=ad_dc_ntvfs"
>
> That should make it easier to see and sort out.
This seems to fix it. Tests to follow, but this could explain a lot of
things, perhaps even our replication failures in repl_move that I was
trying to paper over with a unique prefix.
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
-------------- next part --------------
From c2234c300c678f3e7a1d17bef1fb061c03e89bd6 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 28 May 2018 13:01:18 +1200
Subject: [PATCH] ldb: Save a copy of the index result before calling the
callbacks.
Otherwise Samba modules like subtree_rename can fail as they modify the
index during the callback.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13452
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
lib/ldb/ldb_tdb/ldb_index.c | 80 +++++++++++++++++++++++++++++++++++----------
1 file changed, 62 insertions(+), 18 deletions(-)
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 627424ae8f4..e341cb5426f 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1717,26 +1717,63 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
uint32_t *match_count,
enum key_truncation scope_one_truncation)
{
- struct ldb_context *ldb;
+ struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
struct ldb_message *msg;
struct ldb_message *filtered_msg;
unsigned int i;
+ unsigned int num_keys = 0;
uint8_t previous_guid_key[LTDB_GUID_KEY_SIZE] = {};
- ldb = ldb_module_get_ctx(ac->module);
+ struct guid_tdb_key {
+ uint8_t guid_key[LTDB_GUID_KEY_SIZE];
+ };
+
+ /*
+ * We have to allocate the key list (rather than just walk the
+ * caller supplied list) as the callback could change the list
+ * (by modifying an indexed attribute hosted in the in-memory
+ * index cache!)
+ */
+ TDB_DATA *keys = talloc_array(ac, TDB_DATA, dn_list->count);
+
+ /*
+ * We speculate that the keys will be GUID based and so
+ * pre-fill in enough space for a GUID (avoiding a pile of
+ * small allocations)
+ */
+ struct guid_tdb_key *key_values;
+
+ if (keys == NULL) {
+ return ldb_module_oom(ac->module);
+ }
+
+ if (ltdb->cache->GUID_index_attribute != NULL) {
+ key_values = talloc_array(ac,
+ struct guid_tdb_key,
+ dn_list->count);
+
+ for (i = 0; i < dn_list->count; i++) {
+ keys[i].dptr = key_values[i].guid_key;
+ keys[i].dsize = sizeof(key_values[i].guid_key);
+ }
+ if (key_values == NULL) {
+ return ldb_module_oom(ac->module);
+ }
+ } else {
+ for (i = 0; i < dn_list->count; i++) {
+ keys[i].dptr = NULL;
+ keys[i].dsize = 0;
+ }
+ }
for (i = 0; i < dn_list->count; i++) {
- uint8_t guid_key[LTDB_GUID_KEY_SIZE];
- TDB_DATA tdb_key = {
- .dptr = guid_key,
- .dsize = sizeof(guid_key)
- };
int ret;
- bool matched;
- ret = ltdb_idx_to_key(ac->module, ltdb,
- ac, &dn_list->dn[i],
- &tdb_key);
+ ret = ltdb_idx_to_key(ac->module,
+ ltdb,
+ keys,
+ &dn_list->dn[i],
+ &keys[num_keys]);
if (ret != LDB_SUCCESS) {
return ret;
}
@@ -1753,28 +1790,35 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
* LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK
*/
- if (memcmp(previous_guid_key, tdb_key.dptr,
+ if (memcmp(previous_guid_key,
+ keys[num_keys].dptr,
sizeof(previous_guid_key)) == 0) {
continue;
}
- memcpy(previous_guid_key, tdb_key.dptr,
+ memcpy(previous_guid_key,
+ keys[num_keys].dptr,
sizeof(previous_guid_key));
}
+ num_keys++;
+ }
+
+ /*
+ * Now that the list is a safe copy, send the callbacks
+ */
+ for (i = 0; i < num_keys; i++) {
+ int ret;
+ bool matched;
msg = ldb_msg_new(ac);
if (!msg) {
return LDB_ERR_OPERATIONS_ERROR;
}
-
ret = ltdb_search_key(ac->module, ltdb,
- tdb_key, msg,
+ keys[i], msg,
LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
- if (tdb_key.dptr != guid_key) {
- TALLOC_FREE(tdb_key.dptr);
- }
if (ret == LDB_ERR_NO_SUCH_OBJECT) {
/* the record has disappeared? yes, this can happen */
talloc_free(msg);
--
2.14.3
More information about the samba-technical
mailing list