tdb2: tdb_repack

Move the tdb1_repack() code into the core, make it general, rename to
tdb_repack().

It's generic code: copy database into temporary, wipe it, copy back.

ccan/tdb2/tdb.c

@@ -589,3 +589,64 @@ int tdb_fd(const struct tdb_context *tdb)
 {
 	return tdb->file->fd;
 }
+
+struct traverse_state {
+	enum TDB_ERROR error;
+	struct tdb_context *dest_db;
+};
+
+/*
+  traverse function for repacking
+ */
+static int repack_traverse(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+			   struct traverse_state *state)
+{
+	state->error = tdb_store(state->dest_db, key, data, TDB_INSERT);
+	if (state->error != TDB_SUCCESS) {
+		return -1;
+	}
+	return 0;
+}
+
+enum TDB_ERROR tdb_repack(struct tdb_context *tdb)
+{
+	struct tdb_context *tmp_db;
+	struct traverse_state state;
+
+	state.error = tdb_transaction_start(tdb);
+	if (state.error != TDB_SUCCESS) {
+		return state.error;
+	}
+
+	tmp_db = tdb_open("tmpdb", TDB_INTERNAL, O_RDWR|O_CREAT, 0, NULL);
+	if (tmp_db == NULL) {
+		state.error = tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR,
+					 __location__
+					 " Failed to create tmp_db");
+		tdb_transaction_cancel(tdb);
+		return tdb->last_error = state.error;
+	}
+
+	state.dest_db = tmp_db;
+	if (tdb_traverse(tdb, repack_traverse, &state) < 0) {
+		goto fail;
+	}
+
+	state.error = tdb_wipe_all(tdb);
+	if (state.error != TDB_SUCCESS) {
+		goto fail;
+	}
+
+	state.dest_db = tdb;
+	if (tdb_traverse(tmp_db, repack_traverse, &state) < 0) {
+		goto fail;
+	}
+
+	tdb_close(tmp_db);
+	return tdb_transaction_commit(tdb);
+
+fail:
+	tdb_transaction_cancel(tdb);
+	tdb_close(tmp_db);
+	return state.error;
+}

ccan/tdb2/tdb1.h

@@ -42,8 +42,6 @@ uint64_t tdb1_incompatible_hash(const void *key, size_t len, uint64_t seed, void
 
 /* @} ******************************************************************/
 
-int tdb1_repack(struct tdb_context *tdb);
-
 extern TDB_DATA tdb1_null;
 
 #endif /* tdb1.h */

ccan/tdb2/tdb1_tdb.c

@@ -791,108 +791,6 @@ failed:
 	return -1;
 }
 
-struct traverse_state {
-	enum TDB_ERROR error;
-	struct tdb_context *dest_db;
-};
-
-/*
-  traverse function for repacking
- */
-static int repack_traverse(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, void *private_data)
-{
-	struct traverse_state *state = (struct traverse_state *)private_data;
-	if (tdb1_store(state->dest_db, key, data, TDB_INSERT) != 0) {
-		state->error = state->dest_db->last_error;
-		return -1;
-	}
-	return 0;
-}
-
-/*
-  repack a tdb
- */
-int tdb1_repack(struct tdb_context *tdb)
-{
-	struct tdb_context *tmp_db;
-	struct traverse_state state;
-	union tdb_attribute hsize;
-
-	hsize.base.attr = TDB_ATTRIBUTE_TDB1_HASHSIZE;
-	hsize.base.next = NULL;
-	hsize.tdb1_hashsize.hsize = tdb->tdb1.header.hash_size;
-
-	if (tdb1_transaction_start(tdb) != 0) {
-		tdb_logerr(tdb, tdb->last_error, TDB_LOG_ERROR,
-			   __location__ " Failed to start transaction");
-		return -1;
-	}
-
-	tmp_db = tdb_open("tmpdb", TDB_INTERNAL, O_RDWR|O_CREAT, 0, &hsize);
-	if (tmp_db == NULL) {
-		tdb->last_error = tdb_logerr(tdb, TDB_ERR_OOM, TDB_LOG_ERROR,
-					__location__ " Failed to create tmp_db");
-		tdb1_transaction_cancel(tdb);
-		return -1;
-	}
-
-	state.error = TDB_SUCCESS;
-	state.dest_db = tmp_db;
-
-	if (tdb1_traverse(tdb, repack_traverse, &state) == -1) {
-		tdb_logerr(tdb, tdb->last_error, TDB_LOG_ERROR,
-			   __location__ " Failed to traverse copying out");
-		tdb1_transaction_cancel(tdb);
-		tdb_close(tmp_db);
-		return -1;
-	}
-
-	if (state.error != TDB_SUCCESS) {
-		tdb->last_error = tdb_logerr(tdb, state.error, TDB_LOG_ERROR,
-					__location__ " Error during traversal");
-		tdb1_transaction_cancel(tdb);
-		tdb_close(tmp_db);
-		return -1;
-	}
-
-	if (tdb1_wipe_all(tdb) != 0) {
-		tdb_logerr(tdb, tdb->last_error, TDB_LOG_ERROR,
-			   __location__ " Failed to wipe database\n");
-		tdb1_transaction_cancel(tdb);
-		tdb_close(tmp_db);
-		return -1;
-	}
-
-	state.error = TDB_SUCCESS;
-	state.dest_db = tdb;
-
-	if (tdb1_traverse(tmp_db, repack_traverse, &state) == -1) {
-		tdb_logerr(tdb, tdb->last_error, TDB_LOG_ERROR,
-			   __location__ " Failed to traverse copying back");
-		tdb1_transaction_cancel(tdb);
-		tdb_close(tmp_db);
-		return -1;
-	}
-
-	if (state.error) {
-		tdb->last_error = tdb_logerr(tdb, state.error, TDB_LOG_ERROR,
-					__location__ " Error during second traversal");
-		tdb1_transaction_cancel(tdb);
-		tdb_close(tmp_db);
-		return -1;
-	}
-
-	tdb_close(tmp_db);
-
-	if (tdb1_transaction_commit(tdb) != 0) {
-		tdb_logerr(tdb, tdb->last_error, TDB_LOG_ERROR,
-			   __location__ " Failed to commit");
-		return -1;
-	}
-
-	return 0;
-}
-
 /* Even on files, we can get partial writes due to signals. */
 bool tdb1_write_all(int fd, const void *buf, size_t count)
 {

ccan/tdb2/tdb1_transaction.c

@@ -1153,7 +1153,7 @@ int tdb1_transaction_commit(struct tdb_context *tdb)
 	_tdb1_transaction_cancel(tdb);
 
 	if (need_repack) {
-		return tdb1_repack(tdb);
+		return tdb_repack(tdb);
 	}
 
 	return 0;

ccan/tdb2/tdb2.h

@@ -505,6 +505,16 @@ void tdb_unlockall_read(struct tdb_context *tdb);
  */
 enum TDB_ERROR tdb_wipe_all(struct tdb_context *tdb);
 
+/**
+ * tdb_repack - repack the database
+ * @tdb: the tdb context returned from tdb_open()
+ *
+ * This repacks the database; if it is suffering from a great deal of
+ * fragmentation this might help.  However, it can take twice the
+ * memory of the existing TDB.
+ */
+enum TDB_ERROR tdb_repack(struct tdb_context *tdb);
+
 /**
  * tdb_check - check a TDB for consistency
  * @tdb: the tdb context returned from tdb_open()

ccan/tdb2/test/run-93-repack.c

@@ -0,0 +1,76 @@
+#include "tdb2-source.h"
+#include <ccan/tap/tap.h>
+#include "logging.h"
+
+#define NUM_TESTS 50000
+
+static bool store_all(struct tdb_context *tdb)
+{
+	unsigned int i;
+	struct tdb_data key = { (unsigned char *)&i, sizeof(i) };
+	struct tdb_data dbuf = { (unsigned char *)&i, sizeof(i) };
+
+	for (i = 0; i < NUM_TESTS; i++) {
+		if (tdb_store(tdb, key, dbuf, TDB_INSERT) != TDB_SUCCESS)
+			return false;
+	}
+	return true;
+}
+
+static int mark_entry(struct tdb_context *tdb,
+		      TDB_DATA key, TDB_DATA data, bool found[])
+{
+	unsigned int num;
+
+	if (key.dsize != sizeof(num))
+		return -1;
+	memcpy(&num, key.dptr, key.dsize);
+	if (num >= NUM_TESTS)
+		return -1;
+	if (found[num])
+		return -1;
+	found[num] = true;
+	return 0;
+}
+
+static bool is_all_set(bool found[], unsigned int num)
+{
+	unsigned int i;
+
+	for (i = 0; i < num; i++)
+		if (!found[i])
+			return false;
+	return true;
+}
+
+int main(int argc, char *argv[])
+{
+	unsigned int i;
+	bool found[NUM_TESTS];
+	struct tdb_context *tdb;
+	int flags[] = { TDB_DEFAULT, TDB_NOMMAP,
+			TDB_CONVERT, TDB_NOMMAP|TDB_CONVERT,
+	};
+
+	plan_tests(sizeof(flags) / sizeof(flags[0]) * 6 + 1);
+
+	for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) {
+		tdb = tdb_open("run-93-repack.tdb", flags[i],
+			       O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr);
+		ok1(tdb);
+		if (!tdb)
+			break;
+
+		ok1(store_all(tdb));
+
+		ok1(tdb_repack(tdb) == TDB_SUCCESS);
+		memset(found, 0, sizeof(found));
+		ok1(tdb_check(tdb, NULL, NULL) == TDB_SUCCESS);
+		ok1(tdb_traverse(tdb, mark_entry, found) == NUM_TESTS);
+		ok1(is_all_set(found, NUM_TESTS));
+		tdb_close(tdb);
+	}
+
+	ok1(tap_log_messages == 0);
+	return exit_status();
+}