lib ldb key_value: Pass index cache size
authorGary Lockyer <gary@catalyst.net.nz>
Mon, 1 Apr 2019 01:10:10 +0000 (14:10 +1300)
committerAndrew Bartlett <abartlet@samba.org>
Thu, 4 Apr 2019 06:40:16 +0000 (06:40 +0000)
Pass the index cache size to ldb_kv_index_transaction_start.  This will
allow it to be set for reindex and join operations, where the current
defaults result in a significant performance penalty on large databases.

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
lib/ldb/ldb_key_value/ldb_kv.c
lib/ldb/ldb_key_value/ldb_kv.h
lib/ldb/ldb_key_value/ldb_kv_index.c
lib/ldb/tests/ldb_key_value_test.c [new file with mode: 0644]
lib/ldb/wscript

index d4f896736a298d82699a34f5b2ae9b17a34f54dd..8247ada4256eb5581d71fed14f2ae20ecf2c220c 100644 (file)
@@ -1382,7 +1382,7 @@ static int ldb_kv_start_trans(struct ldb_module *module)
                return ldb_kv->kv_ops->error(ldb_kv);
        }
 
-       ldb_kv_index_transaction_start(module);
+       ldb_kv_index_transaction_start(module, DEFAULT_INDEX_CACHE_SIZE);
 
        ldb_kv->reindex_failed = false;
 
index 5070a588c0073b5b48a3a3bcb1580900da0b4a54..2c08b1db480ba7a468262633333e31dd60e92fa9 100644 (file)
@@ -4,6 +4,8 @@
 #include "tdb.h"
 #include "ldb_module.h"
 
+#ifndef __LDB_KV_H__
+#define __LDB_KV_H__
 struct ldb_kv_private;
 typedef int (*ldb_kv_traverse_fn)(struct ldb_kv_private *ldb_kv,
                                  struct ldb_val key,
@@ -175,6 +177,13 @@ int ldb_kv_check_at_attributes_values(const struct ldb_val *value);
  * The following definitions come from lib/ldb/ldb_key_value/ldb_kv_index.c
  */
 
+/*
+ * The default size of the in memory TDB used to cache index records
+ * The value chosen gives a prime modulo for the hash table and keeps the
+ * tdb memory overhead under 4 kB
+ */
+#define DEFAULT_INDEX_CACHE_SIZE 491
+
 struct ldb_parse_tree;
 
 int ldb_kv_search_indexed(struct ldb_kv_context *ctx, uint32_t *);
@@ -197,7 +206,9 @@ int ldb_kv_index_del_value(struct ldb_module *module,
                           struct ldb_message_element *el,
                           unsigned int v_idx);
 int ldb_kv_reindex(struct ldb_module *module);
-int ldb_kv_index_transaction_start(struct ldb_module *module);
+int ldb_kv_index_transaction_start(
+       struct ldb_module *module,
+       size_t cache_size);
 int ldb_kv_index_transaction_commit(struct ldb_module *module);
 int ldb_kv_index_transaction_cancel(struct ldb_module *module);
 int ldb_kv_key_dn_from_idx(struct ldb_module *module,
@@ -263,3 +274,4 @@ int ldb_kv_init_store(struct ldb_kv_private *ldb_kv,
                      struct ldb_context *ldb,
                      const char *options[],
                      struct ldb_module **_module);
+#endif /* __LDB_KV_H__ */
index e8b2c824b6cccc39fbaf4cceba9310f86bfd0620..003ed714df90d4e21a76b8649e85860d727c4248 100644 (file)
@@ -199,7 +199,9 @@ static unsigned ldb_kv_max_key_length(struct ldb_kv_private *ldb_kv)
 }
 
 /* enable the idxptr mode when transactions start */
-int ldb_kv_index_transaction_start(struct ldb_module *module)
+int ldb_kv_index_transaction_start(
+       struct ldb_module *module,
+       size_t cache_size)
 {
        struct ldb_kv_private *ldb_kv = talloc_get_type(
            ldb_module_get_private(module), struct ldb_kv_private);
@@ -208,7 +210,12 @@ int ldb_kv_index_transaction_start(struct ldb_module *module)
                return ldb_oom(ldb_module_get_ctx(module));
        }
 
-       ldb_kv->idxptr->itdb = tdb_open(NULL, 1000, TDB_INTERNAL, O_RDWR, 0);
+       ldb_kv->idxptr->itdb = tdb_open(
+               NULL,
+               cache_size,
+               TDB_INTERNAL,
+               O_RDWR,
+               0);
        if (ldb_kv->idxptr->itdb == NULL) {
                return LDB_ERR_OPERATIONS_ERROR;
        }
@@ -3105,7 +3112,7 @@ int ldb_kv_reindex(struct ldb_module *module)
         */
        ldb_kv_index_transaction_cancel(module);
 
-       ret = ldb_kv_index_transaction_start(module);
+       ret = ldb_kv_index_transaction_start(module, DEFAULT_INDEX_CACHE_SIZE);
        if (ret != LDB_SUCCESS) {
                return ret;
        }
diff --git a/lib/ldb/tests/ldb_key_value_test.c b/lib/ldb/tests/ldb_key_value_test.c
new file mode 100644 (file)
index 0000000..213405f
--- /dev/null
@@ -0,0 +1,175 @@
+/*
+ * Tests exercising the ldb key value operations.
+ *
+ *  Copyright (C) Andrew Bartlett <abartlet@samba.org> 2019
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ *
+ */
+
+/*
+ *
+ * Tests for the ldb key value layer
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <errno.h>
+#include <unistd.h>
+#include <talloc.h>
+#include <tevent.h>
+#include <string.h>
+#include <ctype.h>
+
+#include <sys/wait.h>
+
+#include "ldb_key_value/ldb_kv.c"
+#include "ldb_key_value/ldb_kv_cache.c"
+#include "ldb_key_value/ldb_kv_index.c"
+#include "ldb_key_value/ldb_kv_search.c"
+
+#define DEFAULT_BE  "tdb"
+
+#ifndef TEST_BE
+#define TEST_BE DEFAULT_BE
+#endif /* TEST_BE */
+
+#define NUM_RECS 1024
+
+
+struct test_ctx {
+};
+
+static int setup(void **state)
+{
+       struct test_ctx *test_ctx;
+
+       test_ctx = talloc_zero(NULL, struct test_ctx);
+       *state = test_ctx;
+       return 0;
+}
+
+static int teardown(void **state)
+{
+       struct test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                         struct test_ctx);
+
+       talloc_free(test_ctx);
+       return 0;
+}
+
+/*
+ * Test that the index cache is opened by ldb_kv_index_transaction_start
+ * and correctly initialised with the passed index cache size.
+ */
+static void test_index_cache_init(void **state)
+{
+       struct test_ctx *test_ctx = talloc_get_type_abort(
+               *state,
+               struct test_ctx);
+       struct ldb_module *module = NULL;
+       struct ldb_kv_private *ldb_kv = NULL;
+       int ret = LDB_SUCCESS;
+
+       module = talloc_zero(test_ctx, struct ldb_module);
+       ldb_kv = talloc_zero(test_ctx, struct ldb_kv_private);
+       ldb_module_set_private(module, ldb_kv);
+
+       ret = ldb_kv_index_transaction_start(module, 191);
+       assert_int_equal(LDB_SUCCESS, ret);
+
+       assert_non_null(ldb_kv->idxptr);
+       assert_non_null(ldb_kv->idxptr->itdb);
+       assert_int_equal(191, tdb_hash_size(ldb_kv->idxptr->itdb));
+
+       TALLOC_FREE(ldb_kv);
+       TALLOC_FREE(module);
+}
+
+static int mock_begin_write(struct ldb_kv_private* ldb_kv) {
+       return LDB_SUCCESS;
+}
+static int mock_abort_write(struct ldb_kv_private* ldb_kv) {
+       return LDB_SUCCESS;
+}
+
+/*
+ * Test that the index cache is set to the default cache size at the start of
+ * a transaction.
+ */
+static void test_default_index_cache_size(void **state)
+{
+       struct test_ctx *test_ctx = talloc_get_type_abort(
+               *state,
+               struct test_ctx);
+       struct ldb_module *module = NULL;
+       struct ldb_kv_private *ldb_kv = NULL;
+       int ret = LDB_SUCCESS;
+       const struct kv_db_ops ops = {
+               .begin_write = mock_begin_write,
+               .abort_write = mock_abort_write
+       };
+
+       module = talloc_zero(test_ctx, struct ldb_module);
+       ldb_kv = talloc_zero(test_ctx, struct ldb_kv_private);
+       ldb_kv->pid = getpid();
+       ldb_kv->kv_ops = &ops;
+       ldb_module_set_private(module, ldb_kv);
+
+       ret = ldb_kv_start_trans(module);
+       assert_int_equal(LDB_SUCCESS, ret);
+
+       assert_int_equal(
+               DEFAULT_INDEX_CACHE_SIZE,
+               tdb_hash_size(ldb_kv->idxptr->itdb));
+
+       ret = ldb_kv_del_trans(module);
+       assert_int_equal(LDB_SUCCESS, ret);
+
+       TALLOC_FREE(ldb_kv);
+       TALLOC_FREE(module);
+}
+
+int main(int argc, const char **argv)
+{
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test_setup_teardown(
+                       test_index_cache_init,
+                       setup,
+                       teardown),
+               cmocka_unit_test_setup_teardown(
+                       test_default_index_cache_size,
+                       setup,
+                       teardown),
+       };
+
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}
index 7891693a0fd01f5348992b4858b2bf7bd3914910..0fdd888f5336d23873d6d490ea3fb5d280c39bff 100644 (file)
@@ -501,6 +501,11 @@ def build(bld):
                          deps='cmocka ldb',
                          install=False)
 
+        bld.SAMBA_BINARY('ldb_key_value_test',
+                         source='tests/ldb_key_value_test.c',
+                         deps='cmocka ldb ldb_tdb_err_map',
+                         install=False)
+
         if bld.CONFIG_SET('HAVE_LMDB'):
             bld.SAMBA_BINARY('ldb_mdb_mod_op_test',
                              source='tests/ldb_mod_op_test.c',
@@ -568,7 +573,8 @@ def test(ctx):
                  'ldb_msg_test',
                  'ldb_tdb_kv_ops_test',
                  'ldb_tdb_test',
-                 'ldb_match_test']
+                 'ldb_match_test',
+                 'ldb_key_value_test']
 
     if env.HAVE_LMDB:
         test_exes += ['ldb_mdb_mod_op_test',