[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Wed Nov 26 11:05:02 MST 2014


The branch, master has been updated
       via  068f9e2 gencache: Request mutexes for gencache_notrans.tdb
       via  f80bbba gencache: Convert gencache.tdb to tdb_wrap
       via  139bd95 gencache: Convert gencache_notrans to use tdb_wrap
       via  35fd2ca s3:gencache: don't use transaction non non-persistent gencache_notrans.tdb
       via  d240cf7 s3:gencache: simply stabilize() a bit more: remove error from state
       via  202ee81 s3:gencache: fix logic in stabilization when deleting a record from stable cache
       via  42b2e5c tdb: Fix tdb_runtime_check_for_robust_mutexes()
      from  ec0c9ad lib: Use tdb_parse_record in gencache_set

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 068f9e26486fbcd36c109df9ada50c9384ba52c5
Author: Christof Schmitt <cs at samba.org>
Date:   Mon Nov 17 14:59:34 2014 -0700

    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>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Wed Nov 26 19:04:11 CET 2014 on sn-devel-104

commit f80bbba2870a80ed421a1a222e430df86028e7c7
Author: Christof Schmitt <cs at samba.org>
Date:   Mon Nov 17 15:44:47 2014 -0700

    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>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 139bd9589ac759b4e7a6ae9aa465320c5fa85d18
Author: Christof Schmitt <cs at samba.org>
Date:   Mon Nov 17 14:30:49 2014 -0700

    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>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 35fd2ca4984b3a1a8bbcb5c1c9e0d724e3c63d80
Author: Michael Adam <obnox at samba.org>
Date:   Wed Jul 2 07:44:04 2014 +0200

    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>

commit d240cf7894f076a2ed2b6bc434f20a93cfbb1ca4
Author: Michael Adam <obnox at samba.org>
Date:   Thu Jun 26 16:56:41 2014 +0200

    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>

commit 202ee81e869f4b51e1f904ef6ac3fb0030edfede
Author: Michael Adam <obnox at samba.org>
Date:   Thu Jun 26 16:37:17 2014 +0200

    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>

commit 42b2e5ca8c9b85e6fce71529bef5d6b3ba4f4a38
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Nov 26 15:35:19 2014 +0000

    tdb: Fix tdb_runtime_check_for_robust_mutexes()
    
    When using exit() instead of _exit(), the child will flush buffered stdout
    (and other stdio) content that it inherited from the parent process. In
    make test, this led to duplicate output from net registry which then
    confused the blackbox selftest.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/tdb/common/mutex.c |   8 +--
 source3/lib/gencache.c | 161 ++++++++++++++++++++++++++++++-------------------
 2 files changed, 103 insertions(+), 66 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/tdb/common/mutex.c b/lib/tdb/common/mutex.c
index bdc4c28..12f89d3 100644
--- a/lib/tdb/common/mutex.c
+++ b/lib/tdb/common/mutex.c
@@ -814,17 +814,17 @@ _PUBLIC_ bool tdb_runtime_check_for_robust_mutexes(void)
 		ret = pthread_mutex_lock(m);
 		nwritten = write(pipe_up[1], &ret, sizeof(ret));
 		if (nwritten != sizeof(ret)) {
-			exit(1);
+			_exit(1);
 		}
 		if (ret != 0) {
-			exit(1);
+			_exit(1);
 		}
 		nread = read(pipe_down[0], &c, 1);
 		if (nread != 1) {
-			exit(1);
+			_exit(1);
 		}
 		/* leave locked */
-		exit(0);
+		_exit(0);
 	}
 	if (tdb_robust_mutex_pid == -1) {
 		goto cleanup_sig_child;
diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 5ee406b..baedf85 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
@@ -36,8 +37,8 @@
 #define BLOB_TYPE "DATA_BLOB"
 #define BLOB_TYPE_LEN 9
 
-static struct tdb_context *cache;
-static struct tdb_context *cache_notrans;
+static struct tdb_wrap *cache;
+static struct tdb_wrap *cache_notrans;
 static int cache_notrans_seqnum;
 
 /**
@@ -71,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.
@@ -88,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));
 		}
@@ -113,25 +117,24 @@ 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;
 	}
 
 	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|
+				      TDB_MUTEX_LOCKING,
+				      open_flags, 0644);
 	if (cache_notrans == NULL) {
 		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);
@@ -320,7 +323,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);
@@ -348,7 +351,7 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 
 	last_stabilize = 0;
 
-	tdb_parse_record(cache_notrans, last_stabilize_key(),
+	tdb_parse_record(cache_notrans->tdb, last_stabilize_key(),
 			 last_stabilize_parser, &last_stabilize);
 
 	if ((last_stabilize
@@ -497,7 +500,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
@@ -515,11 +518,12 @@ 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;
 	}
-	ret = tdb_parse_record(cache, key, gencache_parse_fn, &state);
+	ret = tdb_parse_record(cache->tdb, key, gencache_parse_fn, &state);
 	return (ret == 0);
 }
 
@@ -614,11 +618,13 @@ fail:
 
 struct stabilize_state {
 	bool written;
-	bool error;
 };
 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
  *
@@ -636,9 +642,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,
@@ -648,52 +654,61 @@ 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_transaction_start(cache_notrans);
+
+	res = tdb_lockall(cache_notrans->tdb);
 	if (res != 0) {
-		tdb_transaction_cancel(cache);
-		DEBUG(10, ("Could not start transaction on "
+		tdb_transaction_cancel(cache->tdb);
+		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.error = false;
 	state.written = false;
 
-	res = tdb_traverse(cache_notrans, stabilize_fn, &state);
-	if ((res < 0) || state.error) {
-		tdb_transaction_cancel(cache_notrans);
-		tdb_transaction_cancel(cache);
+	res = tdb_traverse(cache_notrans->tdb, stabilize_fn, &state);
+	if (res < 0) {
+		tdb_unlockall(cache_notrans->tdb);
+		tdb_transaction_cancel(cache->tdb);
 		return false;
 	}
 
 	if (!state.written) {
-		tdb_transaction_cancel(cache_notrans);
-		tdb_transaction_cancel(cache);
+		tdb_unlockall(cache_notrans->tdb);
+		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)));
-		tdb_transaction_cancel(cache_notrans);
+			   "%s\n", tdb_errorstr_compat(cache->tdb)));
+		tdb_unlockall(cache_notrans->tdb);
 		return false;
 	}
 
-	res = tdb_transaction_commit(cache_notrans);
+	res = tdb_traverse(cache_notrans->tdb, wipe_fn, NULL);
 	if (res != 0) {
-		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache)));
+		DEBUG(10, ("tdb_traverse with wipe_fn on gencache_notrans.tdb "
+			  "failed: %s\n",
+			   tdb_errorstr_compat(cache_notrans->tdb)));
+		tdb_unlockall(cache_notrans->tdb);
+		return false;
+	}
+
+	res = tdb_unlockall(cache_notrans->tdb);
+	if (res != 0) {
+		DEBUG(10, ("tdb_unlockall on gencache.tdb failed: "
+			   "%s\n", tdb_errorstr_compat(cache->tdb)));
 		return false;
 	}
 
 	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);
 	}
@@ -717,14 +732,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);
-		if ((res != 0) && (tdb_error(cache) == TDB_ERR_NOEXIST)) {
-			res = 0;
-		} else {
+		res = tdb_delete(cache->tdb, key);
+		if (res == 0) {
 			state->written = true;
+		} 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;
 		}
@@ -732,20 +747,42 @@ 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;
+			   tdb_errorstr_compat(cache->tdb)));
 		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)));
-		state->error = true;
+			   "%s\n", tdb_errorstr_compat(cache_notrans->tdb)));
 		return -1;
 	}
+
 	return 0;
 }
 
+
 /**
  * Get existing entry from the cache file.
  *
@@ -829,7 +866,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;
 	}
 
@@ -884,10 +921,10 @@ 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);
+	tdb_traverse(cache->tdb, gencache_iterate_blobs_fn, &state);
 }
 
 /**


-- 
Samba Shared Repository


More information about the samba-cvs mailing list