From 8f7bf13b96841b38cdc60765a5465be940e1477b Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 1 Apr 2019 14:10:10 +1300 Subject: [PATCH] lib ldb key_value: Pass index cache size 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 Reviewed-by: Andrew Bartlett --- lib/ldb/ldb_key_value/ldb_kv.c | 2 +- lib/ldb/ldb_key_value/ldb_kv.h | 14 ++- lib/ldb/ldb_key_value/ldb_kv_index.c | 13 +- lib/ldb/tests/ldb_key_value_test.c | 175 +++++++++++++++++++++++++++ lib/ldb/wscript | 8 +- 5 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 lib/ldb/tests/ldb_key_value_test.c diff --git a/lib/ldb/ldb_key_value/ldb_kv.c b/lib/ldb/ldb_key_value/ldb_kv.c index d4f896736a29..8247ada4256e 100644 --- a/lib/ldb/ldb_key_value/ldb_kv.c +++ b/lib/ldb/ldb_key_value/ldb_kv.c @@ -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; diff --git a/lib/ldb/ldb_key_value/ldb_kv.h b/lib/ldb/ldb_key_value/ldb_kv.h index 5070a588c007..2c08b1db480b 100644 --- a/lib/ldb/ldb_key_value/ldb_kv.h +++ b/lib/ldb/ldb_key_value/ldb_kv.h @@ -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__ */ diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c index e8b2c824b6cc..003ed714df90 100644 --- a/lib/ldb/ldb_key_value/ldb_kv_index.c +++ b/lib/ldb/ldb_key_value/ldb_kv_index.c @@ -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 index 000000000000..213405f07343 --- /dev/null +++ b/lib/ldb/tests/ldb_key_value_test.c @@ -0,0 +1,175 @@ +/* + * Tests exercising the ldb key value operations. + * + * Copyright (C) Andrew Bartlett 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 . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ + +/* + * + * Tests for the ldb key value layer + */ +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include + +#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); +} diff --git a/lib/ldb/wscript b/lib/ldb/wscript index 7891693a0fd0..0fdd888f5336 100644 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -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', -- 2.34.1