s3:gencache: don't use transaction non non-persistent gencache_notrans.tdb
authorMichael Adam <obnox@samba.org>
Wed, 2 Jul 2014 05:44:04 +0000 (07:44 +0200)
committerVolker Lendecke <vl@samba.org>
Wed, 26 Nov 2014 15:43:04 +0000 (16:43 +0100)
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@samba.org>
Reviewed-by: Christof Schmitt <cs@samba.org>
source3/lib/gencache.c

index 3d165cda3c0c2bc8932df6b46a0deb10236dbc61..9040fc59a12c6c09b12ac820504680adc7f0b5b0 100644 (file)
@@ -618,6 +618,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
  *
@@ -650,10 +653,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;
@@ -663,13 +667,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;
        }
@@ -678,13 +682,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;
        }
@@ -734,14 +746,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.
  *