[PATCHES] Convert gencache to dbwrap to enable mutexes

Christof Schmitt cs at samba.org
Mon Nov 17 16:12:20 MST 2014


On Wed, Nov 05, 2014 at 02:24:12AM +0100, Michael Adam wrote:
> Hi Christof,
> 
> On 2014-11-04 at 15:43 -0700, Christof Schmitt wrote:
> > On Wed, Jul 09, 2014 at 05:25:05PM +0200, Michael Adam wrote:
> > > On 2014-07-08 at 15:14 -0700, Christof Schmitt wrote:
> > > > On Wed, Jun 25, 2014 at 02:25:46PM +0200, Michael Adam wrote:
> > > > > On 2014-06-25 at 14:19 +0200, Volker Lendecke wrote:
> > > > > > On Wed, Jun 25, 2014 at 02:05:08PM +0200, Michael Adam wrote:
> > > > > > > On 2014-06-25 at 13:35 +0200, Volker Lendecke wrote:
> > > > > > > > On Wed, Jun 25, 2014 at 01:18:27PM +0200, Michael Adam wrote:
> > > > > > > > > This would allow to use mutexes correctly
> > > > > > > > > on gencache_notrans. In order to convert
> > > > > > > > > it to dbwrap, we need to add a means of
> > > > > > > > > using allrecord lock via dbrwap (it does
> > > > > > > > > currently not offer it).
> > > > > > > > 
> > > > > > > > ... and I would try to avoid this. I'm not sure I want to
> > > > > > > > think about implementing an allrecord lock for a ctdb
> > > > > > > > database. I'd rather not go through dbwrap for
> > > > > > > > gencache_notrans if that's the only reason to implement a
> > > > > > > > dbwrap_allrecord_lock.
> > > > > > > 
> > > > > > > Right, but we could of course choose to *not* implement it
> > > > > > > in the dbwrap_ctdb case...
> > > > > > > 
> > > > > > > Independently of the choice whether to do the allrecord
> > > > > > > lock throught ctdb or not, do you think the above change
> > > > > > > to gencache_stabilize (on the tdb level) would make sense?
> > > > > > 
> > > > > > Sure. But with your freelist patches, do we still need the
> > > > > > wipe_all? This should be a nice test if freelist compaction
> > > > > > actually works as designed :-)
> > > > > 
> > > > > Ok, sure!
> > > > > But in that case I would do rather a second traverse of the
> > > > > cache_notrans db and do tdb_delete there, in order not to
> > > > > loose entries if the transaction on the persistent db fails.
> > > > > Not as performant as using wipe_all probably, but still. :-)
> > > > 
> > > > Sorry, i got a bit lost in this discussion. Is the current proposal
> > > > changing gencache_notrans to not use transactions, convert it to dbwrap
> > > > and add the mutex check to the local dbwrap codepath?
> > > > 
> > > > I can take a stab at implementing that.
> > > 
> > > Sorry, I lost track myself.
> > > I have a bunch of almost finished patches to the gencache
> > > db code triggered by this discussion that I meant to post a
> > > lot earlier. These remove the transaction from the notrans db.
> > > These could serve as a foundation for your further work.
> > > 
> > > I hope to be able to post them asap, hopefully tonight or tomorrow,
> > > if you still have a little bit of patience with me. :-)
> > 
> > I just found your wip patch to remove transactions from
> > gencache_notrans:
> > http://git.samba.org/?p=obnox/samba/samba-obnox.git;a=commitdiff;h=aa4fddf24f7fa8c2d4feba2adc70417c4f935ede
> 
> Ouch, thanks for digging that up again...
> 
> > Can we work towards getting that patch ready?
> 
> I just rebased and polished that branch:
> 
> https://git.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=master-gencache
> 
> It builds and survives the cache tests.
> Could you check if the patches look ok for you and do
> a few more tests? Then we can move forward.
> 
> Note that instead of doing a tdb_wipe_all(),
> I do a traverse, and delete only those records
> that are not the "@LAST_STABILIZED" key and
> that have a vaild timeout set.
> I'm not 100% certain that this is the best way.
> Maybe this can be solved more elegantly.
> 
> Cheers - Michael
> 
> > After that, we can either
> > convert gencache_notrans to dbwrap, or add mutex support to tdb_wrap and
> > convert gencache_notrans to tdb_wrap as suggested by Volker in
> > https://lists.samba.org/archive/samba-technical/2014-September/102699.html

The patches look good to me. I attached them with my reviewed-by, and
also added patches to convert gencache to tdb_wrap, add a mutex check in
tdb_wrap_open and finally request mutexes for gencache_notrans.

Christof

-------------- next part --------------
From aa90a22967869d6b6e79305b099ab58ee444d438 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 26 Jun 2014 16:37:17 +0200
Subject: [PATCH 1/7] s3:gencache: fix logic in stabilization when deleting a record from stable cache

Set state->written = true in the delete case
if and only if the record has really been deleted.

This does currently not seem to lead to an unneeded
write to the DB, since failure to delete the record
will cause the traverse and hence the transaction
to cancel. But I think this is clearer.

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Christof Schmitt <cs at samba.org>
---
 source3/lib/gencache.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 3192b45..004aa3c 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -710,10 +710,10 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 	}
 	if ((timeout < time(NULL)) || (val.dsize == 0)) {
 		res = tdb_delete(cache, key);
-		if ((res != 0) && (tdb_error(cache) == TDB_ERR_NOEXIST)) {
-			res = 0;
-		} else {
+		if (res == 0) {
 			state->written = true;
+		} else if (tdb_error(cache) == TDB_ERR_NOEXIST) {
+			res = 0;
 		}
 	} else {
 		res = tdb_store(cache, key, val, 0);
-- 
1.7.1


From ecc513fd5bb4ac2dd898f1245a569edb9b3ffe0f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 26 Jun 2014 16:56:41 +0200
Subject: [PATCH 2/7] s3:gencache: simply stabilize() a bit more: remove error from state

state.error is set to true if and only if the traverse
callback returns error (-1), and hence only if the traverse
fails.

Hence the the error state is redundant.

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Christof Schmitt <cs at samba.org>
---
 source3/lib/gencache.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 004aa3c..b6dba13 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -606,7 +606,6 @@ fail:
 
 struct stabilize_state {
 	bool written;
-	bool error;
 };
 static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 			void *priv);
@@ -652,11 +651,10 @@ bool gencache_stabilize(void)
 		return false;
 	}
 
-	state.error = false;
 	state.written = false;
 
 	res = tdb_traverse(cache_notrans, stabilize_fn, &state);
-	if ((res < 0) || state.error) {
+	if (res < 0) {
 		tdb_transaction_cancel(cache_notrans);
 		tdb_transaction_cancel(cache);
 		return false;
@@ -725,14 +723,12 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 	if (res != 0) {
 		DEBUG(10, ("Transfer to gencache.tdb failed: %s\n",
 			   tdb_errorstr_compat(cache)));
-		state->error = true;
 		return -1;
 	}
 
 	if (tdb_delete(cache_notrans, key) != 0) {
 		DEBUG(10, ("tdb_delete from gencache_notrans.tdb failed: "
 			   "%s\n", tdb_errorstr_compat(cache_notrans)));
-		state->error = true;
 		return -1;
 	}
 	return 0;
-- 
1.7.1


From c84e26525bcd4f42f45ca85e3f3409a54ca51361 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 2 Jul 2014 07:44:04 +0200
Subject: [PATCH 3/7] s3:gencache: don't use transaction non non-persistent gencache_notrans.tdb

gencache_notrans.tdb is a non-persistent cache layer above the
persistent gencache.tdb. Despite its name, and despite the
nature of non-persistent tdbs, the current stabilization code
uses a transaction on gencache_notrans.tdb like this:

  transaction_start(cache)
  transaction_start(cache_notrans)
  traverse(cache_notrans, stabilize_fn)
  transaction_commit(cache)
  transaction_commit(cache_notrans)

where stabilze_fn does this on a record:
  1. store it to or delete it from cache
     (depending on the timeout)
  2. delete it from the cache_notrans

This patch changes gencache_notrans.tdb to avoid
transactions by using an all-record lock like this:

  tdb_allrecord_lock(cache_notrans)
  transaction_start(cache)
  traverse(cache_notrans, stabilize_fn_mod)
  transaction_commit(cache)
  traverse(cache_notrans, wipe_fn)
  tdb_wipe_all(cache_notrans)
  tdb_allrecord_unlock(cache_notrans)

with stabilize_fn_mod doing only:
  1. store the record to or delete it from cache
     (depending on the timeout)

and wipe_fn deleting the records from the gencache_notrans db.

This is a step towards making non-persistent-db specific features
like mutex locking usable for gencache_notrans.tdb.

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Christof Schmitt <cs at samba.org>
---
 source3/lib/gencache.c |   52 ++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index b6dba13..766779d 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -610,6 +610,9 @@ struct stabilize_state {
 static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 			void *priv);
 
+static int wipe_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
+		   void *priv);
+
 /**
  * Stabilize gencache
  *
@@ -642,10 +645,11 @@ bool gencache_stabilize(void)
 			   "%s\n", tdb_errorstr_compat(cache)));
 		return false;
 	}
-	res = tdb_transaction_start(cache_notrans);
+
+	res = tdb_lockall(cache_notrans);
 	if (res != 0) {
 		tdb_transaction_cancel(cache);
-		DEBUG(10, ("Could not start transaction on "
+		DEBUG(10, ("Could not get allrecord lock on "
 			   "gencache_notrans.tdb: %s\n",
 			   tdb_errorstr_compat(cache_notrans)));
 		return false;
@@ -655,13 +659,13 @@ bool gencache_stabilize(void)
 
 	res = tdb_traverse(cache_notrans, stabilize_fn, &state);
 	if (res < 0) {
-		tdb_transaction_cancel(cache_notrans);
+		tdb_unlockall(cache_notrans);
 		tdb_transaction_cancel(cache);
 		return false;
 	}
 
 	if (!state.written) {
-		tdb_transaction_cancel(cache_notrans);
+		tdb_unlockall(cache_notrans);
 		tdb_transaction_cancel(cache);
 		return true;
 	}
@@ -670,13 +674,21 @@ bool gencache_stabilize(void)
 	if (res != 0) {
 		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
 			   "%s\n", tdb_errorstr_compat(cache)));
-		tdb_transaction_cancel(cache_notrans);
+		tdb_unlockall(cache_notrans);
 		return false;
 	}
 
-	res = tdb_transaction_commit(cache_notrans);
+	res = tdb_traverse(cache_notrans, wipe_fn, NULL);
 	if (res != 0) {
-		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
+		DEBUG(10, ("tdb_traverse with wipe_fn on gencache_notrans.tdb "
+			  "failed: %s\n", tdb_errorstr_compat(cache_notrans)));
+		tdb_unlockall(cache_notrans);
+		return false;
+	}
+
+	res = tdb_unlockall(cache_notrans);
+	if (res != 0) {
+		DEBUG(10, ("tdb_unlockall on gencache.tdb failed: "
 			   "%s\n", tdb_errorstr_compat(cache)));
 		return false;
 	}
@@ -726,14 +738,38 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 		return -1;
 	}
 
-	if (tdb_delete(cache_notrans, key) != 0) {
+	return 0;
+}
+
+static int wipe_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
+		   void *priv)
+{
+	int res;
+	bool ok;
+	time_t timeout;
+
+	res = tdb_data_cmp(key, last_stabilize_key());
+	if (res == 0) {
+		return 0;
+	}
+
+	ok = gencache_pull_timeout((char *)val.dptr, &timeout, NULL);
+	if (!ok) {
+		DEBUG(10, ("Ignoring invalid entry\n"));
+		return 0;
+	}
+
+	res = tdb_delete(tdb, key);
+	if (res != 0) {
 		DEBUG(10, ("tdb_delete from gencache_notrans.tdb failed: "
 			   "%s\n", tdb_errorstr_compat(cache_notrans)));
 		return -1;
 	}
+
 	return 0;
 }
 
+
 /**
  * Get existing entry from the cache file.
  *
-- 
1.7.1


From a33d67a2ca2925cc8d7bb824eae7cdb28a12c103 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 17 Nov 2014 15:05:24 -0700
Subject: [PATCH 4/7] tdb_wrap: Add check for tdb mutexes

Allow callers of tdb_wrap_open to safely request mutexes. The flag will
be ignored on systems that don't support mutexes. The patch has been
sugggested by metze in
https://lists.samba.org/archive/samba-technical/2014-September/102704.html

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 lib/tdb_wrap/tdb_wrap.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/lib/tdb_wrap/tdb_wrap.c b/lib/tdb_wrap/tdb_wrap.c
index f2f32d1..2b75426 100644
--- a/lib/tdb_wrap/tdb_wrap.c
+++ b/lib/tdb_wrap/tdb_wrap.c
@@ -149,6 +149,12 @@ struct tdb_wrap *tdb_wrap_open(TALLOC_CTX *mem_ctx,
 	}
 
 	if (w == NULL) {
+		if (tdb_flags & TDB_MUTEX_LOCKING) {
+			if (!tdb_runtime_check_for_robust_mutexes()) {
+				tdb_flags &= ~TDB_MUTEX_LOCKING;
+			}
+		}
+
 		w = tdb_wrap_private_open(result, name, hash_size, tdb_flags,
 					  open_flags, mode);
 	} else {
-- 
1.7.1


From 70cc82f24f7d730b478e7d37929e83f1d4d97355 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 17 Nov 2014 14:30:49 -0700
Subject: [PATCH 5/7] gencache: Convert gencache_notrans to use tdb_wrap

This allows using on the mutex check in tdb_wrap.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/lib/gencache.c |   53 +++++++++++++++++++++++++----------------------
 1 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 766779d..572e6ba 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -25,6 +25,7 @@
 #include "system/filesys.h"
 #include "system/glob.h"
 #include "util_tdb.h"
+#include "tdb_wrap/tdb_wrap.h"
 #include "../lib/util/memcache.h"
 
 #undef  DBGC_CLASS
@@ -37,7 +38,7 @@
 #define BLOB_TYPE_LEN 9
 
 static struct tdb_context *cache;
-static struct tdb_context *cache_notrans;
+static struct tdb_wrap *cache_notrans;
 static int cache_notrans_seqnum;
 
 /**
@@ -120,12 +121,12 @@ static bool gencache_init(void)
 
 	DEBUG(5, ("Opening cache file at %s\n", cache_fname));
 
-	cache_notrans = tdb_open_log(cache_fname, 0,
-				     TDB_CLEAR_IF_FIRST|
-				     TDB_INCOMPATIBLE_HASH|
-				     TDB_SEQNUM|
-				     TDB_NOSYNC,
-				     open_flags, 0644);
+	cache_notrans = tdb_wrap_open(NULL, cache_fname, 0,
+				      TDB_CLEAR_IF_FIRST|
+				      TDB_INCOMPATIBLE_HASH|
+				      TDB_SEQNUM|
+				      TDB_NOSYNC,
+				      open_flags, 0644);
 	if (cache_notrans == NULL) {
 		DEBUG(5, ("Opening %s failed: %s\n", cache_fname,
 			  strerror(errno)));
@@ -310,7 +311,7 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 		   timeout > time(NULL) ? "ahead" : "in the past"));
 
 	ret = tdb_store_bystring(
-		cache_notrans, keystr,
+		cache_notrans->tdb, keystr,
 		make_tdb_data((uint8_t *)val, talloc_array_length(val)),
 		0);
 	TALLOC_FREE(val);
@@ -337,7 +338,7 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 	 */
 
 	last_stabilize = 0;
-	databuf = tdb_fetch_compat(cache_notrans, last_stabilize_key());
+	databuf = tdb_fetch_compat(cache_notrans->tdb, last_stabilize_key());
 	if ((databuf.dptr != NULL)
 	    && (databuf.dptr[databuf.dsize-1] == '\0')) {
 		last_stabilize = atoi((char *)databuf.dptr);
@@ -489,7 +490,7 @@ bool gencache_parse(const char *keystr,
 		 * Make sure that nobody has changed the gencache behind our
 		 * back.
 		 */
-		int current_seqnum = tdb_get_seqnum(cache_notrans);
+		int current_seqnum = tdb_get_seqnum(cache_notrans->tdb);
 		if (current_seqnum == cache_notrans_seqnum) {
 			/*
 			 * Ok, our memcache is still current, use it without
@@ -507,7 +508,8 @@ bool gencache_parse(const char *keystr,
 
 	state.is_memcache = false;
 
-	ret = tdb_parse_record(cache_notrans, key, gencache_parse_fn, &state);
+	ret = tdb_parse_record(cache_notrans->tdb, key,
+			       gencache_parse_fn, &state);
 	if (ret == 0) {
 		return true;
 	}
@@ -646,26 +648,26 @@ bool gencache_stabilize(void)
 		return false;
 	}
 
-	res = tdb_lockall(cache_notrans);
+	res = tdb_lockall(cache_notrans->tdb);
 	if (res != 0) {
 		tdb_transaction_cancel(cache);
 		DEBUG(10, ("Could not get allrecord lock on "
 			   "gencache_notrans.tdb: %s\n",
-			   tdb_errorstr_compat(cache_notrans)));
+			   tdb_errorstr_compat(cache_notrans->tdb)));
 		return false;
 	}
 
 	state.written = false;
 
-	res = tdb_traverse(cache_notrans, stabilize_fn, &state);
+	res = tdb_traverse(cache_notrans->tdb, stabilize_fn, &state);
 	if (res < 0) {
-		tdb_unlockall(cache_notrans);
+		tdb_unlockall(cache_notrans->tdb);
 		tdb_transaction_cancel(cache);
 		return false;
 	}
 
 	if (!state.written) {
-		tdb_unlockall(cache_notrans);
+		tdb_unlockall(cache_notrans->tdb);
 		tdb_transaction_cancel(cache);
 		return true;
 	}
@@ -674,19 +676,20 @@ bool gencache_stabilize(void)
 	if (res != 0) {
 		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
 			   "%s\n", tdb_errorstr_compat(cache)));
-		tdb_unlockall(cache_notrans);
+		tdb_unlockall(cache_notrans->tdb);
 		return false;
 	}
 
-	res = tdb_traverse(cache_notrans, wipe_fn, NULL);
+	res = tdb_traverse(cache_notrans->tdb, wipe_fn, NULL);
 	if (res != 0) {
 		DEBUG(10, ("tdb_traverse with wipe_fn on gencache_notrans.tdb "
-			  "failed: %s\n", tdb_errorstr_compat(cache_notrans)));
-		tdb_unlockall(cache_notrans);
+			  "failed: %s\n",
+			   tdb_errorstr_compat(cache_notrans->tdb)));
+		tdb_unlockall(cache_notrans->tdb);
 		return false;
 	}
 
-	res = tdb_unlockall(cache_notrans);
+	res = tdb_unlockall(cache_notrans->tdb);
 	if (res != 0) {
 		DEBUG(10, ("tdb_unlockall on gencache.tdb failed: "
 			   "%s\n", tdb_errorstr_compat(cache)));
@@ -695,7 +698,7 @@ bool gencache_stabilize(void)
 
 	now = talloc_asprintf(talloc_tos(), "%d", (int)time(NULL));
 	if (now != NULL) {
-		tdb_store(cache_notrans, last_stabilize_key(),
+		tdb_store(cache_notrans->tdb, last_stabilize_key(),
 			  string_term_tdb_data(now), 0);
 		TALLOC_FREE(now);
 	}
@@ -762,7 +765,7 @@ static int wipe_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 	res = tdb_delete(tdb, key);
 	if (res != 0) {
 		DEBUG(10, ("tdb_delete from gencache_notrans.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache_notrans)));
+			   "%s\n", tdb_errorstr_compat(cache_notrans->tdb)));
 		return -1;
 	}
 
@@ -853,7 +856,7 @@ static int gencache_iterate_blobs_fn(struct tdb_context *tdb, TDB_DATA key,
 	if (tdb_data_cmp(key, last_stabilize_key()) == 0) {
 		return 0;
 	}
-	if (state->in_persistent && tdb_exists(cache_notrans, key)) {
+	if (state->in_persistent && tdb_exists(cache_notrans->tdb, key)) {
 		return 0;
 	}
 
@@ -908,7 +911,7 @@ void gencache_iterate_blobs(void (*fn)(const char *key, DATA_BLOB value,
 	state.private_data = private_data;
 
 	state.in_persistent = false;
-	tdb_traverse(cache_notrans, gencache_iterate_blobs_fn, &state);
+	tdb_traverse(cache_notrans->tdb, gencache_iterate_blobs_fn, &state);
 
 	state.in_persistent = true;
 	tdb_traverse(cache, gencache_iterate_blobs_fn, &state);
-- 
1.7.1


From 45c1a5919bb501ef8b1d2972c20f4db792d9c1b4 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 17 Nov 2014 15:44:47 -0700
Subject: [PATCH 6/7] gencache: Convert gencache.tdb to tdb_wrap

This change is not strictly necessary, but for consistency both gencache
tdbs are now opened through tdb_wrap.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/lib/gencache.c |   61 ++++++++++++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 572e6ba..11df9c4 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -37,7 +37,7 @@
 #define BLOB_TYPE "DATA_BLOB"
 #define BLOB_TYPE_LEN 9
 
-static struct tdb_context *cache;
+static struct tdb_wrap *cache;
 static struct tdb_wrap *cache_notrans;
 static int cache_notrans_seqnum;
 
@@ -72,12 +72,14 @@ static bool gencache_init(void)
 
 	DEBUG(5, ("Opening cache file at %s\n", cache_fname));
 
-	cache = tdb_open_log(cache_fname, 0, TDB_DEFAULT|TDB_INCOMPATIBLE_HASH, open_flags, 0644);
+	cache = tdb_wrap_open(NULL, cache_fname, 0,
+			      TDB_DEFAULT|TDB_INCOMPATIBLE_HASH,
+			      open_flags, 0644);
 	if (cache) {
 		int ret;
-		ret = tdb_check(cache, NULL, NULL);
+		ret = tdb_check(cache->tdb, NULL, NULL);
 		if (ret != 0) {
-			tdb_close(cache);
+			TALLOC_FREE(cache);
 
 			/*
 			 * Retry with CLEAR_IF_FIRST.
@@ -89,18 +91,19 @@ static bool gencache_init(void)
 			 * CLEAR_IF_FIRST databases, so lets use it here to
 			 * clean up a broken database.
 			 */
-			cache = tdb_open_log(cache_fname, 0,
-					     TDB_DEFAULT|
-					     TDB_INCOMPATIBLE_HASH|
-					     TDB_CLEAR_IF_FIRST,
-					     open_flags, 0644);
+			cache = tdb_wrap_open(NULL, cache_fname, 0,
+					      TDB_DEFAULT|
+					      TDB_INCOMPATIBLE_HASH|
+					      TDB_CLEAR_IF_FIRST,
+					      open_flags, 0644);
 		}
 	}
 
 	if (!cache && (errno == EACCES)) {
 		open_flags = O_RDONLY;
-		cache = tdb_open_log(cache_fname, 0, TDB_DEFAULT|TDB_INCOMPATIBLE_HASH, open_flags,
-				     0644);
+		cache = tdb_wrap_open(NULL, cache_fname, 0,
+				      TDB_DEFAULT|TDB_INCOMPATIBLE_HASH,
+				      open_flags, 0644);
 		if (cache) {
 			DEBUG(5, ("gencache_init: Opening cache file %s read-only.\n", cache_fname));
 		}
@@ -114,8 +117,7 @@ static bool gencache_init(void)
 
 	cache_fname = lock_path("gencache_notrans.tdb");
 	if (cache_fname == NULL) {
-		tdb_close(cache);
-		cache = NULL;
+		TALLOC_FREE(cache);
 		return false;
 	}
 
@@ -131,8 +133,7 @@ static bool gencache_init(void)
 		DEBUG(5, ("Opening %s failed: %s\n", cache_fname,
 			  strerror(errno)));
 		TALLOC_FREE(cache_fname);
-		tdb_close(cache);
-		cache = NULL;
+		TALLOC_FREE(cache);
 		return false;
 	}
 	TALLOC_FREE(cache_fname);
@@ -513,7 +514,7 @@ bool gencache_parse(const char *keystr,
 	if (ret == 0) {
 		return true;
 	}
-	ret = tdb_parse_record(cache, key, gencache_parse_fn, &state);
+	ret = tdb_parse_record(cache->tdb, key, gencache_parse_fn, &state);
 	return (ret == 0);
 }
 
@@ -632,9 +633,9 @@ bool gencache_stabilize(void)
 		return false;
 	}
 
-	res = tdb_transaction_start_nonblock(cache);
+	res = tdb_transaction_start_nonblock(cache->tdb);
 	if (res != 0) {
-		if (tdb_error(cache) == TDB_ERR_NOLOCK)
+		if (tdb_error(cache->tdb) == TDB_ERR_NOLOCK)
 		{
 			/*
 			 * Someone else already does the stabilize,
@@ -644,13 +645,13 @@ bool gencache_stabilize(void)
 		}
 
 		DEBUG(10, ("Could not start transaction on gencache.tdb: "
-			   "%s\n", tdb_errorstr_compat(cache)));
+			   "%s\n", tdb_errorstr_compat(cache->tdb)));
 		return false;
 	}
 
 	res = tdb_lockall(cache_notrans->tdb);
 	if (res != 0) {
-		tdb_transaction_cancel(cache);
+		tdb_transaction_cancel(cache->tdb);
 		DEBUG(10, ("Could not get allrecord lock on "
 			   "gencache_notrans.tdb: %s\n",
 			   tdb_errorstr_compat(cache_notrans->tdb)));
@@ -662,20 +663,20 @@ bool gencache_stabilize(void)
 	res = tdb_traverse(cache_notrans->tdb, stabilize_fn, &state);
 	if (res < 0) {
 		tdb_unlockall(cache_notrans->tdb);
-		tdb_transaction_cancel(cache);
+		tdb_transaction_cancel(cache->tdb);
 		return false;
 	}
 
 	if (!state.written) {
 		tdb_unlockall(cache_notrans->tdb);
-		tdb_transaction_cancel(cache);
+		tdb_transaction_cancel(cache->tdb);
 		return true;
 	}
 
-	res = tdb_transaction_commit(cache);
+	res = tdb_transaction_commit(cache->tdb);
 	if (res != 0) {
 		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache)));
+			   "%s\n", tdb_errorstr_compat(cache->tdb)));
 		tdb_unlockall(cache_notrans->tdb);
 		return false;
 	}
@@ -692,7 +693,7 @@ bool gencache_stabilize(void)
 	res = tdb_unlockall(cache_notrans->tdb);
 	if (res != 0) {
 		DEBUG(10, ("tdb_unlockall on gencache.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache)));
+			   "%s\n", tdb_errorstr_compat(cache->tdb)));
 		return false;
 	}
 
@@ -722,14 +723,14 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 		return 0;
 	}
 	if ((timeout < time(NULL)) || (val.dsize == 0)) {
-		res = tdb_delete(cache, key);
+		res = tdb_delete(cache->tdb, key);
 		if (res == 0) {
 			state->written = true;
-		} else if (tdb_error(cache) == TDB_ERR_NOEXIST) {
+		} else if (tdb_error(cache->tdb) == TDB_ERR_NOEXIST) {
 			res = 0;
 		}
 	} else {
-		res = tdb_store(cache, key, val, 0);
+		res = tdb_store(cache->tdb, key, val, 0);
 		if (res == 0) {
 			state->written = true;
 		}
@@ -737,7 +738,7 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 
 	if (res != 0) {
 		DEBUG(10, ("Transfer to gencache.tdb failed: %s\n",
-			   tdb_errorstr_compat(cache)));
+			   tdb_errorstr_compat(cache->tdb)));
 		return -1;
 	}
 
@@ -914,7 +915,7 @@ void gencache_iterate_blobs(void (*fn)(const char *key, DATA_BLOB value,
 	tdb_traverse(cache_notrans->tdb, gencache_iterate_blobs_fn, &state);
 
 	state.in_persistent = true;
-	tdb_traverse(cache, gencache_iterate_blobs_fn, &state);
+	tdb_traverse(cache->tdb, gencache_iterate_blobs_fn, &state);
 }
 
 /**
-- 
1.7.1


From 846671b89ac9aae541084dde06a689643e8e7e76 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 17 Nov 2014 14:59:34 -0700
Subject: [PATCH 7/7] gencache: Request mutexes for gencache_notrans.tdb

The check in tdb_wrap ensures that mutexes are only used on systems that
properly support them.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/lib/gencache.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 11df9c4..0716cfe 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -127,7 +127,8 @@ static bool gencache_init(void)
 				      TDB_CLEAR_IF_FIRST|
 				      TDB_INCOMPATIBLE_HASH|
 				      TDB_SEQNUM|
-				      TDB_NOSYNC,
+				      TDB_NOSYNC|
+				      TDB_MUTEX_LOCKING,
 				      open_flags, 0644);
 	if (cache_notrans == NULL) {
 		DEBUG(5, ("Opening %s failed: %s\n", cache_fname,
-- 
1.7.1



More information about the samba-technical mailing list