diff options
author | Andrew Bartlett <abartlet@samba.org> | 2017-03-31 17:34:13 +1300 |
---|---|---|
committer | Stefan Metzmacher <metze@samba.org> | 2017-07-02 17:35:18 +0200 |
commit | f80076fe43f18404575cb9ac86ecfc637127c4ba (patch) | |
tree | 2debcbea0387cd9f8096a5eec52dcc3e133d4c2a /lib | |
parent | b87af322d5fea8bd8280057996ce542c999bcaef (diff) | |
download | samba-f80076fe43f18404575cb9ac86ecfc637127c4ba.tar.gz |
tdb: Remove locking from tdb_traverse_read()
This restores the original intent of tdb_traverse_read() in
7dd31288a701d772e45b1960ac4ce4cc1be782ed
This is needed to avoid a deadlock with tdb_lockall() and the
transaction start, as ldb_tdb should take the allrecord lock during a
search (which calls tdb_traverse), and can otherwise deadlock against
a transaction starting in another process
We add a test to show that a transaction can now start while a read
traverse is in progress
This allows more operations to happen in parallel. The blocking point
is moved to the prepare commit.
This in turn permits a roughly doubling of unindexed search
performance, because currently ldb_tdb omits to take the lock due to
an unrelated bug, but taking the allrecord lock triggers the
above-mentioned deadlock.
This behaviour was added in 251aaafe3a9213118ac3a92def9ab2104c40d12a for
Solaris 10 in 2005. But the run-fcntl-deadlock test works also on Solaris 10,
see https://lists.samba.org/archive/samba-technical/2017-April/119876.html.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/tdb/common/traverse.c | 10 | ||||
-rw-r--r-- | lib/tdb/test/run-nested-traverse.c | 31 |
2 files changed, 28 insertions, 13 deletions
diff --git a/lib/tdb/common/traverse.c b/lib/tdb/common/traverse.c index f33ef34ab8d..f62306e5560 100644 --- a/lib/tdb/common/traverse.c +++ b/lib/tdb/common/traverse.c @@ -244,7 +244,7 @@ out: /* - a read style traverse - temporarily marks the db read only + a read style traverse - temporarily marks each record read only */ _PUBLIC_ int tdb_traverse_read(struct tdb_context *tdb, tdb_traverse_func fn, void *private_data) @@ -252,19 +252,11 @@ _PUBLIC_ int tdb_traverse_read(struct tdb_context *tdb, struct tdb_traverse_lock tl = { NULL, 0, 0, F_RDLCK }; int ret; - /* we need to get a read lock on the transaction lock here to - cope with the lock ordering semantics of solaris10 */ - if (tdb_transaction_lock(tdb, F_RDLCK, TDB_LOCK_WAIT)) { - return -1; - } - tdb->traverse_read++; tdb_trace(tdb, "tdb_traverse_read_start"); ret = tdb_traverse_internal(tdb, fn, private_data, &tl); tdb->traverse_read--; - tdb_transaction_unlock(tdb, F_RDLCK); - return ret; } diff --git a/lib/tdb/test/run-nested-traverse.c b/lib/tdb/test/run-nested-traverse.c index 22ee3e2a2a6..aeaa0859793 100644 --- a/lib/tdb/test/run-nested-traverse.c +++ b/lib/tdb/test/run-nested-traverse.c @@ -41,7 +41,30 @@ static int traverse2(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, return 0; } -static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, +static int traverse1r(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, + void *p) +{ + ok1(correct_key(key)); + ok1(correct_data(data)); + ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb)) + == SUCCESS); + ok1(external_agent_operation(agent, STORE, tdb_name(tdb)) + == SUCCESS); + ok1(external_agent_operation(agent, TRANSACTION_COMMIT, tdb_name(tdb)) + == WOULD_HAVE_BLOCKED); + tdb_traverse(tdb, traverse2, NULL); + + /* That should *not* release the all-records lock! */ + ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb)) + == SUCCESS); + ok1(external_agent_operation(agent, STORE, tdb_name(tdb)) + == SUCCESS); + ok1(external_agent_operation(agent, TRANSACTION_COMMIT, tdb_name(tdb)) + == WOULD_HAVE_BLOCKED); + return 0; +} + +static int traverse1w(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, void *p) { ok1(correct_key(key)); @@ -50,7 +73,7 @@ static int traverse1(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, == WOULD_HAVE_BLOCKED); tdb_traverse(tdb, traverse2, NULL); - /* That should *not* release the transaction lock! */ + /* That should *not* release the all-records lock! */ ok1(external_agent_operation(agent, TRANSACTION_START, tdb_name(tdb)) == WOULD_HAVE_BLOCKED); return 0; @@ -80,8 +103,8 @@ int main(int argc, char *argv[]) data.dsize = strlen("world"); ok1(tdb_store(tdb, key, data, TDB_INSERT) == 0); - tdb_traverse(tdb, traverse1, NULL); - tdb_traverse_read(tdb, traverse1, NULL); + tdb_traverse(tdb, traverse1w, NULL); + tdb_traverse_read(tdb, traverse1r, NULL); tdb_close(tdb); return exit_status(); |