[PATCHES] Convert gencache to dbwrap to enable mutexes
Christof Schmitt
cs at samba.org
Fri Nov 21 13:08:23 MST 2014
On Mon, Nov 17, 2014 at 04:12:20PM -0700, Christof Schmitt wrote:
> 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.
Just noticed that Volker already pushed the tdb_wrap check for mutexes.
Here are the remaining six patches that apply on top of the current
master branch. Someone would need to review the gencache changes i made.
Regards,
Christof
-------------- next part --------------
From 0357979022a7372eed06a8b48add9328aa87c0cc 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/6] 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 cca5b0eba9e873fbedb12d3e7f7a65166c9aa4b6 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/6] 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 c5255ed712c298fa6145ddb35fdff6dcf99f28d5 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/6] 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 84d9898dbdc9ec74489ffcaf63036bed9f24dd2f 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 4/6] 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 92c3ed4e0a65903c511a01ab9f66c11bfacc8302 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 5/6] 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 48eb7b6190fe3c223c5ee8092966b8f8276ff35a 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 6/6] 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