The Samba-Bugzilla – Attachment 15933 Details for
Bug 14330
LMDB databases can grow without bounds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for V4.11 (v1)
Bug-14330-4.11-V1.patch (text/plain), 21.45 KB, created by
Gary Lockyer
on 2020-04-20 04:24:28 UTC
(
hide
)
Description:
Patch for V4.11 (v1)
Filename:
MIME Type:
Creator:
Gary Lockyer
Created:
2020-04-20 04:24:28 UTC
Size:
21.45 KB
patch
obsolete
>From e71c0a00b8c60aef6fb94565e466738a0cd83706 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Mon, 16 Mar 2020 15:18:12 +1300 >Subject: [PATCH 1/3] ldb tests: Confirm lmdb free list handling > >Add cmocka tests to confirm lmdb's handling of the free list. > >As a result of lmdb's MVCC (Multiversion Concurrency Control) long >running read transactions or stale readers (read transactions where the >process exited without ending the transaction) can cause the database to >run out of space. > >Items in the free list are only reused when they would not be visible in >a read transaction. So long running read transactions prevent entries >in the free list being reused, and the database can run out of space. > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >(cherry picked from commit fc13304d1575ad6bc6e04cdb3eedf46d3c3678c7) >--- > lib/ldb/tests/ldb_lmdb_free_list_test.c | 656 ++++++++++++++++++++++++ > lib/ldb/wscript | 8 +- > 2 files changed, 663 insertions(+), 1 deletion(-) > create mode 100644 lib/ldb/tests/ldb_lmdb_free_list_test.c > >diff --git a/lib/ldb/tests/ldb_lmdb_free_list_test.c b/lib/ldb/tests/ldb_lmdb_free_list_test.c >new file mode 100644 >index 00000000000..fe78e3ab702 >--- /dev/null >+++ b/lib/ldb/tests/ldb_lmdb_free_list_test.c >@@ -0,0 +1,656 @@ >+/* >+ * Copyright (C) Catalyst.Net Ltd 2020 >+ * >+ * 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/>. >+ * >+ */ >+ >+/* >+ * Tests confirming lmdb's handling of the free space list in the presence >+ * of active and stale readers. A stale reader is a process that opens a >+ * read lock and then exits without releasing the lock. >+ * >+ * lmdb uses MVCC to maintain databased consistency, new copies of updated >+ * records are written to the database. The old entries are only >+ * reused when they are no longer referenced in a read transaction. >+ * >+ * The tests all update a single record multiple times >+ * >+ * If there is a read transaction or a stale reader lmdb will report >+ * out of space. >+ * >+ * If no read transaction and no stale reader, lmdb reclaims space from the >+ * free list. >+ */ >+ >+/* >+ * 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. >+ * >+ */ >+ >+#include <stdarg.h> >+#include <stddef.h> >+#include <stdint.h> >+#include <setjmp.h> >+#include <cmocka.h> >+ >+#include <errno.h> >+#include <unistd.h> >+#include <talloc.h> >+#include <tevent.h> >+#include <ldb.h> >+#include <ldb_module.h> >+#include <ldb_private.h> >+#include <string.h> >+#include <ctype.h> >+ >+#include <sys/wait.h> >+ >+#include "ldb_tdb/ldb_tdb.h" >+#include "ldb_key_value/ldb_kv.h" >+ >+#define DEFAULT_BE "mdb" >+ >+#ifndef TEST_BE >+#define TEST_BE DEFAULT_BE >+#endif /* TEST_BE */ >+ >+const int RECORD_SIZE = 6144; >+const int ITERATIONS = 3; >+ >+struct test_ctx { >+ struct tevent_context *ev; >+ struct ldb_context *ldb; >+ >+ const char *dbfile; >+ const char *lockfile; /* lockfile is separate */ >+ >+ const char *dbpath; >+}; >+ >+static void unlink_old_db(struct test_ctx *test_ctx) >+{ >+ int ret; >+ >+ errno = 0; >+ ret = unlink(test_ctx->lockfile); >+ if (ret == -1 && errno != ENOENT) { >+ fail(); >+ } >+ >+ errno = 0; >+ ret = unlink(test_ctx->dbfile); >+ if (ret == -1 && errno != ENOENT) { >+ fail(); >+ } >+} >+ >+static int noconn_setup(void **state) >+{ >+ struct test_ctx *test_ctx; >+ >+ test_ctx = talloc_zero(NULL, struct test_ctx); >+ assert_non_null(test_ctx); >+ >+ test_ctx->ev = tevent_context_init(test_ctx); >+ assert_non_null(test_ctx->ev); >+ >+ test_ctx->ldb = ldb_init(test_ctx, test_ctx->ev); >+ assert_non_null(test_ctx->ldb); >+ >+ test_ctx->dbfile = talloc_strdup(test_ctx, "lmdb_free_list_test.ldb"); >+ assert_non_null(test_ctx->dbfile); >+ >+ test_ctx->lockfile = >+ talloc_asprintf(test_ctx, "%s-lock", test_ctx->dbfile); >+ assert_non_null(test_ctx->lockfile); >+ >+ test_ctx->dbpath = >+ talloc_asprintf(test_ctx, TEST_BE "://%s", test_ctx->dbfile); >+ assert_non_null(test_ctx->dbpath); >+ >+ unlink_old_db(test_ctx); >+ *state = test_ctx; >+ return 0; >+} >+ >+static int noconn_teardown(void **state) >+{ >+ struct test_ctx *test_ctx = >+ talloc_get_type_abort(*state, struct test_ctx); >+ >+ unlink_old_db(test_ctx); >+ talloc_free(test_ctx); >+ return 0; >+} >+ >+static int setup(void **state) >+{ >+ struct test_ctx *test_ctx; >+ int ret; >+ struct ldb_ldif *ldif; >+ const char *index_ldif = "dn: @INDEXLIST\n" >+ "@IDXGUID: objectUUID\n" >+ "@IDX_DN_GUID: GUID\n" >+ "\n"; >+ /* >+ * Use a 64KiB DB for this test >+ */ >+ const char *options[] = {"lmdb_env_size:65536", NULL}; >+ >+ noconn_setup((void **)&test_ctx); >+ >+ ret = ldb_connect(test_ctx->ldb, test_ctx->dbpath, 0, options); >+ assert_int_equal(ret, 0); >+ >+ while ((ldif = ldb_ldif_read_string(test_ctx->ldb, &index_ldif))) { >+ ret = ldb_add(test_ctx->ldb, ldif->msg); >+ assert_int_equal(ret, LDB_SUCCESS); >+ } >+ *state = test_ctx; >+ return 0; >+} >+ >+static int teardown(void **state) >+{ >+ struct test_ctx *test_ctx = >+ talloc_get_type_abort(*state, struct test_ctx); >+ noconn_teardown((void **)&test_ctx); >+ return 0; >+} >+ >+static struct ldb_kv_private *get_ldb_kv(struct ldb_context *ldb) >+{ >+ void *data = NULL; >+ struct ldb_kv_private *ldb_kv = NULL; >+ >+ data = ldb_module_get_private(ldb->modules); >+ assert_non_null(data); >+ >+ ldb_kv = talloc_get_type(data, struct ldb_kv_private); >+ assert_non_null(ldb_kv); >+ >+ return ldb_kv; >+} >+ >+static int parse(struct ldb_val key, struct ldb_val data, void *private_data) >+{ >+ struct ldb_val *read = private_data; >+ >+ /* Yes, we leak this. That is OK */ >+ read->data = talloc_size(NULL, data.length); >+ assert_non_null(read->data); >+ >+ memcpy(read->data, data.data, data.length); >+ read->length = data.length; >+ return LDB_SUCCESS; >+} >+ >+/* >+ * This test has the same structure as the test_free_list_read_lock >+ * except the parent process does not keep the read lock open while the >+ * child process is performing an update. >+ */ >+static void test_free_list_no_read_lock(void **state) >+{ >+ int ret; >+ struct test_ctx *test_ctx = >+ talloc_get_type_abort(*state, struct test_ctx); >+ struct ldb_kv_private *ldb_kv = get_ldb_kv(test_ctx->ldb); >+ struct ldb_val key; >+ struct ldb_val val; >+ >+ const char *KEY1 = "KEY01"; >+ >+ /* >+ * Pipes etc to co-ordinate the processes >+ */ >+ int to_child[2]; >+ int to_parent[2]; >+ char buf[2]; >+ pid_t pid; >+ size_t i; >+ >+ TALLOC_CTX *tmp_ctx; >+ tmp_ctx = talloc_new(test_ctx); >+ assert_non_null(tmp_ctx); >+ >+ ret = pipe(to_child); >+ assert_int_equal(ret, 0); >+ ret = pipe(to_parent); >+ assert_int_equal(ret, 0); >+ /* >+ * Now fork a new process >+ */ >+ >+ pid = fork(); >+ if (pid == 0) { >+ /* >+ * Child process >+ */ >+ >+ struct ldb_context *ldb = NULL; >+ close(to_child[1]); >+ close(to_parent[0]); >+ >+ /* >+ * Wait for the parent to get ready. >+ */ >+ ret = read(to_child[0], buf, 2); >+ assert_int_equal(ret, 2); >+ >+ ldb = ldb_init(test_ctx, test_ctx->ev); >+ assert_non_null(ldb); >+ >+ ret = ldb_connect(ldb, test_ctx->dbpath, 0, NULL); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ldb_kv = get_ldb_kv(ldb); >+ assert_non_null(ldb_kv); >+ /* >+ * Add a record to the database >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ val.data = talloc_zero_size(tmp_ctx, RECORD_SIZE); >+ assert_non_null(val.data); >+ memset(val.data, 'x', RECORD_SIZE); >+ val.length = RECORD_SIZE; >+ /* >+ * Do more iterations than when a read lock, stale reader >+ * active to confirm that the space is being re-used. >+ */ >+ for (i = 0; i < ITERATIONS * 10; i++) { >+ ret = ldb_kv->kv_ops->begin_write(ldb_kv); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->store(ldb_kv, key, val, 0); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->finish_write(ldb_kv); >+ assert_int_equal(ret, LDB_SUCCESS); >+ } >+ >+ /* >+ * Signal the parent that we've done the updates >+ */ >+ ret = write(to_parent[1], "GO", 2); >+ assert_int_equal(ret, 2); >+ exit(0); >+ } >+ >+ close(to_child[0]); >+ close(to_parent[1]); >+ >+ /* >+ * Begin a read transaction >+ */ >+ ret = ldb_kv->kv_ops->lock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ /* >+ * Now close it >+ */ >+ ret = ldb_kv->kv_ops->unlock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ /* >+ * Signal the child process >+ */ >+ ret = write(to_child[1], "GO", 2); >+ assert_int_equal(2, ret); >+ >+ /* >+ * Wait for the child process to update the record >+ */ >+ ret = read(to_parent[0], buf, 2); >+ assert_int_equal(2, ret); >+ >+ /* >+ * Begin a read transaction >+ */ >+ ret = ldb_kv->kv_ops->lock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ /* >+ * read the record >+ * and close the transaction >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ >+ ret = ldb_kv->kv_ops->fetch_and_parse(ldb_kv, key, parse, &val); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->unlock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ close(to_child[1]); >+ close(to_parent[0]); >+ TALLOC_FREE(tmp_ctx); >+} >+ >+/* >+ * This test has the same structure as the test_free_list_read_lock >+ * except the parent process keeps the read lock open while the >+ * child process is performing an update. >+ */ >+static void test_free_list_read_lock(void **state) >+{ >+ int ret; >+ struct test_ctx *test_ctx = >+ talloc_get_type_abort(*state, struct test_ctx); >+ struct ldb_kv_private *ldb_kv = get_ldb_kv(test_ctx->ldb); >+ struct ldb_val key; >+ struct ldb_val val; >+ >+ const char *KEY1 = "KEY01"; >+ >+ /* >+ * Pipes etc to co-ordinate the processes >+ */ >+ int to_child[2]; >+ int to_parent[2]; >+ char buf[2]; >+ pid_t pid; >+ size_t i; >+ >+ TALLOC_CTX *tmp_ctx; >+ tmp_ctx = talloc_new(test_ctx); >+ assert_non_null(tmp_ctx); >+ >+ ret = pipe(to_child); >+ assert_int_equal(ret, 0); >+ ret = pipe(to_parent); >+ assert_int_equal(ret, 0); >+ /* >+ * Now fork a new process >+ */ >+ >+ pid = fork(); >+ if (pid == 0) { >+ /* >+ * Child process >+ */ >+ >+ struct ldb_context *ldb = NULL; >+ close(to_child[1]); >+ close(to_parent[0]); >+ >+ /* >+ * Wait for the transaction to start >+ */ >+ ret = read(to_child[0], buf, 2); >+ assert_int_equal(ret, 2); >+ >+ ldb = ldb_init(test_ctx, test_ctx->ev); >+ assert_non_null(ldb); >+ >+ ret = ldb_connect(ldb, test_ctx->dbpath, 0, NULL); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ldb_kv = get_ldb_kv(ldb); >+ assert_non_null(ldb_kv); >+ /* >+ * Add a record to the database >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ val.data = talloc_zero_size(tmp_ctx, RECORD_SIZE); >+ assert_non_null(val.data); >+ memset(val.data, 'x', RECORD_SIZE); >+ val.length = RECORD_SIZE; >+ for (i = 0; i < ITERATIONS; i++) { >+ ret = ldb_kv->kv_ops->begin_write(ldb_kv); >+ assert_int_equal(ret, 0); >+ ret = ldb_kv->kv_ops->store(ldb_kv, key, val, 0); >+ if (ret == LDB_ERR_BUSY && i > 0) { >+ int rc = ldb_kv->kv_ops->abort_write(ldb_kv); >+ assert_int_equal(rc, LDB_SUCCESS); >+ break; >+ } >+ assert_int_equal(ret, LDB_SUCCESS); >+ ret = ldb_kv->kv_ops->finish_write(ldb_kv); >+ assert_int_equal(ret, LDB_SUCCESS); >+ } >+ assert_int_equal(ret, LDB_ERR_BUSY); >+ assert_int_not_equal(i, 0); >+ >+ /* >+ * Begin a read transaction >+ */ >+ ret = ldb_kv->kv_ops->lock_read(ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ /* >+ * read the record >+ * and close the transaction >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ >+ ret = ldb_kv->kv_ops->fetch_and_parse(ldb_kv, key, parse, &val); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->unlock_read(ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ /* >+ * Signal the the parent that we've done the update >+ */ >+ ret = write(to_parent[1], "GO", 2); >+ assert_int_equal(ret, 2); >+ exit(0); >+ } >+ >+ close(to_child[0]); >+ close(to_parent[1]); >+ >+ /* >+ * Begin a read transaction >+ */ >+ ret = ldb_kv->kv_ops->lock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ /* >+ * Signal the child process >+ */ >+ ret = write(to_child[1], "GO", 2); >+ assert_int_equal(ret, 2); >+ >+ /* >+ * Wait for the child process to update the record >+ */ >+ ret = read(to_parent[0], buf, 2); >+ assert_int_equal(ret, 2); >+ >+ /* >+ * read the record >+ * and close the transaction >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ >+ ret = ldb_kv->kv_ops->fetch_and_parse(ldb_kv, key, parse, &val); >+ assert_int_equal(ret, LDB_ERR_NO_SUCH_OBJECT); >+ ret = ldb_kv->kv_ops->unlock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, 0); >+ >+ close(to_child[1]); >+ close(to_parent[0]); >+ TALLOC_FREE(tmp_ctx); >+} >+ >+/* >+ * This tests forks a child process that opens a read lock and then >+ * exits. This results in a stale reader entry in the lmdb lock file. >+ */ >+static void test_free_list_stale_reader(void **state) >+{ >+ int ret; >+ struct test_ctx *test_ctx = >+ talloc_get_type_abort(*state, struct test_ctx); >+ struct ldb_kv_private *ldb_kv = get_ldb_kv(test_ctx->ldb); >+ struct ldb_val key; >+ struct ldb_val val; >+ >+ const char *KEY1 = "KEY01"; >+ >+ /* >+ * Pipes etc to co-ordinate the processes >+ */ >+ int to_child[2]; >+ int to_parent[2]; >+ char buf[2]; >+ pid_t pid; >+ size_t i; >+ >+ TALLOC_CTX *tmp_ctx; >+ tmp_ctx = talloc_new(test_ctx); >+ assert_non_null(tmp_ctx); >+ >+ ret = pipe(to_child); >+ assert_int_equal(ret, 0); >+ ret = pipe(to_parent); >+ assert_int_equal(ret, 0); >+ /* >+ * Now fork a new process >+ */ >+ >+ pid = fork(); >+ if (pid == 0) { >+ /* >+ * Child process >+ */ >+ >+ struct ldb_context *ldb = NULL; >+ close(to_child[1]); >+ close(to_parent[0]); >+ >+ /* >+ * Wait for the parent to get ready >+ */ >+ ret = read(to_child[0], buf, 2); >+ assert_int_equal(ret, 2); >+ >+ ldb = ldb_init(test_ctx, test_ctx->ev); >+ assert_non_null(ldb); >+ >+ ret = ldb_connect(ldb, test_ctx->dbpath, 0, NULL); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ldb_kv = get_ldb_kv(ldb); >+ assert_non_null(ldb_kv); >+ >+ /* >+ * Begin a read transaction >+ */ >+ ret = ldb_kv->kv_ops->lock_read(ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ /* >+ * Now exit with out releasing the read lock >+ * this will result in a stale entry in the >+ * read lock table. >+ */ >+ >+ exit(0); >+ } >+ >+ close(to_child[0]); >+ close(to_parent[1]); >+ >+ /* >+ * Tell the child to start >+ */ >+ ret = write(to_child[1], "GO", 2); >+ assert_int_equal(ret, 2); >+ >+ close(to_child[1]); >+ close(to_parent[0]); >+ >+ /* >+ * Now wait for the child process to complete >+ */ >+ waitpid(pid, NULL, 0); >+ >+ /* >+ * Add a record to the database >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ val.data = talloc_zero_size(tmp_ctx, RECORD_SIZE); >+ assert_non_null(val.data); >+ memset(val.data, 'x', RECORD_SIZE); >+ val.length = RECORD_SIZE; >+ for (i = 0; i < ITERATIONS; i++) { >+ ret = ldb_kv->kv_ops->begin_write(ldb_kv); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->store(ldb_kv, key, val, 0); >+ if (ret == LDB_ERR_BUSY && i > 0) { >+ int rc = ldb_kv->kv_ops->abort_write(ldb_kv); >+ assert_int_equal(rc, LDB_SUCCESS); >+ break; >+ } >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->finish_write(ldb_kv); >+ assert_int_equal(ret, LDB_SUCCESS); >+ } >+ assert_int_equal(ret, LDB_ERR_BUSY); >+ assert_int_not_equal(i, 0); >+ >+ /* >+ * Begin a read transaction >+ */ >+ ret = ldb_kv->kv_ops->lock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ /* >+ * read the record >+ * and close the transaction >+ */ >+ key.data = (uint8_t *)talloc_strdup(tmp_ctx, KEY1); >+ key.length = strlen(KEY1) + 1; >+ >+ ret = ldb_kv->kv_ops->fetch_and_parse(ldb_kv, key, parse, &val); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ ret = ldb_kv->kv_ops->unlock_read(test_ctx->ldb->modules); >+ assert_int_equal(ret, LDB_SUCCESS); >+ >+ TALLOC_FREE(tmp_ctx); >+} >+ >+int main(int argc, const char **argv) >+{ >+ const struct CMUnitTest tests[] = { >+ cmocka_unit_test_setup_teardown( >+ test_free_list_no_read_lock, setup, teardown), >+ cmocka_unit_test_setup_teardown( >+ test_free_list_read_lock, setup, teardown), >+ cmocka_unit_test_setup_teardown( >+ test_free_list_stale_reader, setup, teardown), >+ }; >+ >+ return cmocka_run_group_tests(tests, NULL, NULL); >+} >diff --git a/lib/ldb/wscript b/lib/ldb/wscript >index c3f2f42b8c1..9b276467d3a 100644 >--- a/lib/ldb/wscript >+++ b/lib/ldb/wscript >@@ -551,6 +551,11 @@ def build(bld): > deps='cmocka ldb', > install=False) > >+ bld.SAMBA_BINARY('ldb_lmdb_free_list_test', >+ source='tests/ldb_lmdb_free_list_test.c', >+ cflags='-DTEST_BE=\"mdb\" -DTEST_LMDB=1', >+ deps='cmocka ldb', >+ install=False) > # > # We rely on the versions of the ldb_key_value functions included > # in ldb_key_value_sub_txn_test.c taking priority over the versions >@@ -630,7 +635,8 @@ def test(ctx): > # fit > 4G of data into the DB), it would fill up the disk on > # many of our test instances > 'ldb_mdb_kv_ops_test', >- 'ldb_key_value_sub_txn_mdb_test'] >+ 'ldb_key_value_sub_txn_mdb_test', >+ 'ldb_lmdb_free_list_test'] > else: > test_exes += ['ldb_no_lmdb_test'] > >-- >2.17.1 > > >From 38cb53ebafeb67bcbb2b59658db1f2427743830f Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Mon, 30 Mar 2020 12:08:30 +1300 >Subject: [PATCH 2/3] lib ldb: lmdb clear stale readers on write txn start > >In use process failures and Bind9 shut downs leave stale entries in the >lmdb reader table. This can result in lmdb filling it's database file, as >the free list can not be reclaimed due to the stale reader. > >In this fix we call mdb_reader_check at the start of each transaction, >to free any stale readers. As the default maximum number of readers is >127, this should not impact on performance to any great extent. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14330 > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> > >Autobuild-User(master): Andrew Bartlett <abartlet@samba.org> >Autobuild-Date(master): Tue Mar 31 01:26:07 UTC 2020 on sn-devel-184 > >(cherry picked from commit 89041a6d18a1d091ea713e6986cac5ca66c2b481) >--- > lib/ldb/ldb_mdb/ldb_mdb.c | 17 +++++++++++++++++ > lib/ldb/tests/ldb_lmdb_free_list_test.c | 7 ++++++- > 2 files changed, 23 insertions(+), 1 deletion(-) > >diff --git a/lib/ldb/ldb_mdb/ldb_mdb.c b/lib/ldb/ldb_mdb/ldb_mdb.c >index 6c679c214b8..1187aba578a 100644 >--- a/lib/ldb/ldb_mdb/ldb_mdb.c >+++ b/lib/ldb/ldb_mdb/ldb_mdb.c >@@ -641,6 +641,23 @@ static int lmdb_transaction_start(struct ldb_kv_private *ldb_kv) > return LDB_ERR_PROTOCOL_ERROR; > } > >+ /* >+ * Clear out any stale readers >+ */ >+ { >+ int stale; >+ mdb_reader_check(lmdb->env, &stale); >+ if (stale > 0) { >+ ldb_debug( >+ lmdb->ldb, >+ LDB_DEBUG_ERROR, >+ "LMDB Stale readers, deleted (%d)", >+ stale); >+ } >+ } >+ >+ >+ > ltx_head = lmdb_private_trans_head(lmdb); > > tx_parent = lmdb_trans_get_tx(ltx_head); >diff --git a/lib/ldb/tests/ldb_lmdb_free_list_test.c b/lib/ldb/tests/ldb_lmdb_free_list_test.c >index fe78e3ab702..9b295460730 100644 >--- a/lib/ldb/tests/ldb_lmdb_free_list_test.c >+++ b/lib/ldb/tests/ldb_lmdb_free_list_test.c >@@ -617,7 +617,12 @@ static void test_free_list_stale_reader(void **state) > ret = ldb_kv->kv_ops->finish_write(ldb_kv); > assert_int_equal(ret, LDB_SUCCESS); > } >- assert_int_equal(ret, LDB_ERR_BUSY); >+ /* >+ * We now do an explicit clear of stale readers at the start of a >+ * write transaction so should not get LDB_ERR_BUSY any more >+ * assert_int_equal(ret, LDB_ERR_BUSY); >+ */ >+ assert_int_equal(ret, LDB_SUCCESS); > assert_int_not_equal(i, 0); > > /* >-- >2.17.1 > > >From 89aa46b8b5815a659ad2bd47226a0d0a29228ca5 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Wed, 1 Apr 2020 08:22:08 +1300 >Subject: [PATCH 3/3] lib ldb: lmdb init var before calling mdb_reader_check > >Initilalise "stale" to zero before passing a pointer to it to >mdb_reader_check. > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andreas Schneider <asn@samba.org> > >Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org> >Autobuild-Date(master): Tue Apr 7 12:29:00 UTC 2020 on sn-devel-184 > >(cherry picked from commit d1f4002b914efb31aa34a59e7c93d80f3174727c) >--- > lib/ldb/ldb_mdb/ldb_mdb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/lib/ldb/ldb_mdb/ldb_mdb.c b/lib/ldb/ldb_mdb/ldb_mdb.c >index 1187aba578a..f0a418d07ee 100644 >--- a/lib/ldb/ldb_mdb/ldb_mdb.c >+++ b/lib/ldb/ldb_mdb/ldb_mdb.c >@@ -645,7 +645,7 @@ static int lmdb_transaction_start(struct ldb_kv_private *ldb_kv) > * Clear out any stale readers > */ > { >- int stale; >+ int stale = 0; > mdb_reader_check(lmdb->env, &stale); > if (stale > 0) { > ldb_debug( >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
metze
:
review-
gary
:
ci-passed+
Actions:
View
Attachments on
bug 14330
:
15932
|
15933
|
15949
|
15950