The branch, master has been updated
via 36cfa7b tdb: make sure we skip over recovery area correctly.
via cb88418 tdb_expand: limit the expansion with huge records
via 094ab60 tdb: tdb_repack() only when it's worthwhile.
via 6aa72da tdb: fix transaction recovery area for converted tdbs.
from dfb490d selftest Consolidate server wall clock time limits
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 36cfa7b79e36d880cdbf24d0769558be44d0edda
Author: Rusty Russell <[email protected]>
Date: Tue Apr 19 21:00:59 2011 +0930
tdb: make sure we skip over recovery area correctly.
If it's really the recovery area, we can trust the rec_len field, and
don't have to go groping for bitpatterns.
Signed-off-by: Rusty Russell <[email protected]>
Autobuild-User: Rusty Russell <[email protected]>
Autobuild-Date: Tue Apr 19 14:15:22 CEST 2011 on sn-devel-104
commit cb884186a55c9ef8aca6ee48b16423b3c881e689
Author: Simo Sorce <[email protected]>
Date: Mon Apr 18 22:15:11 2011 +0930
tdb_expand: limit the expansion with huge records
ldb can create huge records when saving indexes.
Limit the tdb expansion to avoid consuming a lot of memory for
no good reason if the record being saved is huge.
commit 094ab60053bcc0bc3542af8144e394d83270053e
Author: Rusty Russell <[email protected]>
Date: Mon Apr 18 22:15:11 2011 +0930
tdb: tdb_repack() only when it's worthwhile.
tdb_repack() is expensive and consumes memory, so we can spend some
effort to see if it's worthwhile. In particular, tdbbackup doesn't
need to repack: it started with an empty database!
Signed-off-by: Rusty Russell <[email protected]>
commit 6aa72dae8fc341de5b497f831ded1f8f519fa8fb
Author: Rusty Russell <[email protected]>
Date: Mon Apr 18 22:15:11 2011 +0930
tdb: fix transaction recovery area for converted tdbs.
This is why macros are dangerous; these were converting the pointers, not
the
things pointed to!
Signed-off-by: Rusty Russell <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
lib/tdb/common/io.c | 25 ++++++++++--
lib/tdb/common/summary.c | 13 +++++-
lib/tdb/common/tdb_private.h | 4 ++
lib/tdb/common/transaction.c | 87 +++++++++++++++++++++++++++++++-----------
4 files changed, 99 insertions(+), 30 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/tdb/common/io.c b/lib/tdb/common/io.c
index 058ca6c..78bbf2e 100644
--- a/lib/tdb/common/io.c
+++ b/lib/tdb/common/io.c
@@ -299,7 +299,7 @@ static int tdb_expand_file(struct tdb_context *tdb,
tdb_off_t size, tdb_off_t ad
int tdb_expand(struct tdb_context *tdb, tdb_off_t size)
{
struct tdb_record rec;
- tdb_off_t offset, new_size;
+ tdb_off_t offset, new_size, top_size, map_size;
if (tdb_lock(tdb, -1, F_WRLCK) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "lock failed in tdb_expand\n"));
@@ -309,10 +309,25 @@ int tdb_expand(struct tdb_context *tdb, tdb_off_t size)
/* must know about any previous expansions by another process */
tdb->methods->tdb_oob(tdb, tdb->map_size + 1, 1);
- /* always make room for at least 100 more records, and at
- least 25% more space. Round the database up to a multiple
- of the page size */
- new_size = MAX(tdb->map_size + size*100, tdb->map_size * 1.25);
+ /* limit size in order to avoid using up huge amounts of memory for
+ * in memory tdbs if an oddball huge record creeps in */
+ if (size > 100 * 1024) {
+ top_size = tdb->map_size + size * 2;
+ } else {
+ top_size = tdb->map_size + size * 100;
+ }
+
+ /* always make room for at least top_size more records, and at
+ least 25% more space. if the DB is smaller than 100MiB,
+ otherwise grow it by 10% only. */
+ if (tdb->map_size > 100 * 1024 * 1024) {
+ map_size = tdb->map_size * 1.10;
+ } else {
+ map_size = tdb->map_size * 1.25;
+ }
+
+ /* Round the database up to a multiple of the page size */
+ new_size = MAX(top_size, map_size);
size = TDB_ALIGN(new_size, tdb->page_size) - tdb->map_size;
if (!(tdb->flags & TDB_INTERNAL))
diff --git a/lib/tdb/common/summary.c b/lib/tdb/common/summary.c
index b222a59..171a1a2 100644
--- a/lib/tdb/common/summary.c
+++ b/lib/tdb/common/summary.c
@@ -86,12 +86,13 @@ static size_t get_hash_length(struct tdb_context *tdb,
unsigned int i)
_PUBLIC_ char *tdb_summary(struct tdb_context *tdb)
{
- tdb_off_t off;
+ tdb_off_t off, rec_off;
struct tally freet, keys, data, dead, extra, hash, uncoal;
struct tdb_record rec;
char *ret = NULL;
bool locked;
size_t len, unc = 0;
+ struct tdb_record recovery;
/* Read-only databases use no locking at all: it's best-effort.
* We may have a write lock already, so skip that case too. */
@@ -103,6 +104,10 @@ _PUBLIC_ char *tdb_summary(struct tdb_context *tdb)
locked = true;
}
+ if (tdb_recovery_area(tdb, tdb->methods, &rec_off, &recovery) != 0) {
+ goto unlock;
+ }
+
tally_init(&freet);
tally_init(&keys);
tally_init(&data);
@@ -135,7 +140,11 @@ _PUBLIC_ char *tdb_summary(struct tdb_context *tdb)
case TDB_RECOVERY_INVALID_MAGIC:
case 0x42424242:
unc++;
- rec.rec_len = tdb_dead_space(tdb, off) - sizeof(rec);
+ /* If it's a valid recovery, we can trust rec_len. */
+ if (off != rec_off) {
+ rec.rec_len = tdb_dead_space(tdb, off)
+ - sizeof(rec);
+ }
/* Fall through */
case TDB_DEAD_MAGIC:
tally_add(&dead, rec.rec_len);
diff --git a/lib/tdb/common/tdb_private.h b/lib/tdb/common/tdb_private.h
index 0186fb9..140d4ec 100644
--- a/lib/tdb/common/tdb_private.h
+++ b/lib/tdb/common/tdb_private.h
@@ -238,6 +238,10 @@ void tdb_release_transaction_locks(struct tdb_context
*tdb);
int tdb_transaction_lock(struct tdb_context *tdb, int ltype,
enum tdb_lock_flags lockflags);
int tdb_transaction_unlock(struct tdb_context *tdb, int ltype);
+int tdb_recovery_area(struct tdb_context *tdb,
+ const struct tdb_methods *methods,
+ tdb_off_t *recovery_offset,
+ struct tdb_record *rec);
int tdb_allrecord_lock(struct tdb_context *tdb, int ltype,
enum tdb_lock_flags flags, bool upgradable);
int tdb_allrecord_unlock(struct tdb_context *tdb, int ltype, bool mark_lock);
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index c49de38..e4573cb 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -138,8 +138,8 @@ struct tdb_transaction {
/* old file size before transaction */
tdb_len_t old_map_size;
- /* we should re-pack on commit */
- bool need_repack;
+ /* did we expand in this transaction */
+ bool expanded;
};
@@ -403,7 +403,7 @@ static int transaction_expand_file(struct tdb_context *tdb,
tdb_off_t size,
return -1;
}
- tdb->transaction->need_repack = true;
+ tdb->transaction->expanded = true;
return 0;
}
@@ -658,6 +658,34 @@ static tdb_len_t tdb_recovery_size(struct tdb_context *tdb)
return recovery_size;
}
+int tdb_recovery_area(struct tdb_context *tdb,
+ const struct tdb_methods *methods,
+ tdb_off_t *recovery_offset,
+ struct tdb_record *rec)
+{
+ if (tdb_ofs_read(tdb, TDB_RECOVERY_HEAD, recovery_offset) == -1) {
+ return -1;
+ }
+
+ if (*recovery_offset == 0) {
+ rec->rec_len = 0;
+ return 0;
+ }
+
+ if (methods->tdb_read(tdb, *recovery_offset, rec, sizeof(*rec),
+ DOCONV()) == -1) {
+ return -1;
+ }
+
+ /* ignore invalid recovery regions: can happen in crash */
+ if (rec->magic != TDB_RECOVERY_MAGIC &&
+ rec->magic != TDB_RECOVERY_INVALID_MAGIC) {
+ *recovery_offset = 0;
+ rec->rec_len = 0;
+ }
+ return 0;
+}
+
/*
allocate the recovery area, or use an existing recovery area if it is
large enough
@@ -671,25 +699,11 @@ static int tdb_recovery_allocate(struct tdb_context *tdb,
const struct tdb_methods *methods = tdb->transaction->io_methods;
tdb_off_t recovery_head;
- if (tdb_ofs_read(tdb, TDB_RECOVERY_HEAD, &recovery_head) == -1) {
+ if (tdb_recovery_area(tdb, methods, &recovery_head, &rec) == -1) {
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_recovery_allocate: failed
to read recovery head\n"));
return -1;
}
- rec.rec_len = 0;
-
- if (recovery_head != 0) {
- if (methods->tdb_read(tdb, recovery_head, &rec, sizeof(rec),
DOCONV()) == -1) {
- TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_recovery_allocate:
failed to read recovery record\n"));
- return -1;
- }
- /* ignore invalid recovery regions: can happen in crash */
- if (rec.magic != TDB_RECOVERY_MAGIC &&
- rec.magic != TDB_RECOVERY_INVALID_MAGIC) {
- recovery_head = 0;
- }
- }
-
*recovery_size = tdb_recovery_size(tdb);
if (recovery_head != 0 && *recovery_size <= rec.rec_len) {
@@ -786,7 +800,7 @@ static int transaction_setup_recovery(struct tdb_context
*tdb,
rec->data_len = recovery_size;
rec->rec_len = recovery_max_size;
rec->key_len = old_map_size;
- CONVERT(rec);
+ CONVERT(*rec);
/* build the recovery data into a single blob to allow us to do a single
large write, which should be more efficient */
@@ -833,7 +847,9 @@ static int transaction_setup_recovery(struct tdb_context
*tdb,
/* and the tailer */
tailer = sizeof(*rec) + recovery_max_size;
memcpy(p, &tailer, 4);
- CONVERT(p);
+ if (DOCONV()) {
+ tdb_convert(p, 4);
+ }
/* write the recovery data to the recovery area */
if (methods->tdb_write(tdb, recovery_offset, data, sizeof(*rec) +
recovery_size) == -1) {
@@ -981,6 +997,28 @@ _PUBLIC_ int tdb_transaction_prepare_commit(struct
tdb_context *tdb)
return _tdb_transaction_prepare_commit(tdb);
}
+/* A repack is worthwhile if the largest is less than half total free. */
+static bool repack_worthwhile(struct tdb_context *tdb)
+{
+ tdb_off_t ptr;
+ struct tdb_record rec;
+ tdb_len_t total = 0, largest = 0;
+
+ if (tdb_ofs_read(tdb, FREELIST_TOP, &ptr) == -1) {
+ return false;
+ }
+
+ while (ptr != 0 && tdb_rec_free_read(tdb, ptr, &rec) == 0) {
+ total += rec.rec_len;
+ if (rec.rec_len > largest) {
+ largest = rec.rec_len;
+ }
+ ptr = rec.next;
+ }
+
+ return total > largest * 2;
+}
+
/*
commit the current transaction
*/
@@ -988,7 +1026,7 @@ _PUBLIC_ int tdb_transaction_commit(struct tdb_context
*tdb)
{
const struct tdb_methods *methods;
int i;
- bool need_repack;
+ bool need_repack = false;
if (tdb->transaction == NULL) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_commit: no
transaction\n"));
@@ -1056,6 +1094,11 @@ _PUBLIC_ int tdb_transaction_commit(struct tdb_context
*tdb)
SAFE_FREE(tdb->transaction->blocks[i]);
}
+ /* Do this before we drop lock or blocks. */
+ if (tdb->transaction->expanded) {
+ need_repack = repack_worthwhile(tdb);
+ }
+
SAFE_FREE(tdb->transaction->blocks);
tdb->transaction->num_blocks = 0;
@@ -1079,8 +1122,6 @@ _PUBLIC_ int tdb_transaction_commit(struct tdb_context
*tdb)
utime(tdb->name, NULL);
#endif
- need_repack = tdb->transaction->need_repack;
-
/* use a transaction cancel to free memory and remove the
transaction locks */
_tdb_transaction_cancel(tdb);
--
Samba Shared Repository
