[SCM] Samba Shared Repository - branch v3-5-test updated
Michael Adam
obnox at samba.org
Tue Nov 10 04:44:10 MST 2009
The branch, v3-5-test has been updated
via b58c7d9... s3:registry: add an extra check for dsize==0 to regdb_fetch_keys_internal()
via 3fb7db8... s3:registry: add safety check for return value of tdb_unpack to regdb_fetch_keys_internal()
via bb939df... s3:dbwrap_ctdb: add debug message to transaction_fetch_start()
via 8b7189f... s3:dbwrap_ctdb: split combined check in two and add descriptive debug
via 056c237... s3:dbwrap_ctdb: fix race condition with concurrent transactions on the same node.
via 8296188... s3:configure: add a check for the new CTDB_CONTROL_TRANS2_ACTIVE
via ef3ab7b... s3:dbwrap_ctdb: add new db_ctdb_transaction_active() that calls CTDB_CONTROL_TRANS2_COMMIT
via 7b37b8a... s3:dbwrap_ctdb: fix a race in starting concurrent transactions on a single node
via f50ef43... s3:dbwrap_ctdb: use db_ctdb_ltdb_fetch() inside db_ctdb_transaction_fetch_start
via 6f004b2... s3:dbwrap_ctdb: use db_ctdb_ltdb_fetch() inside db_ctdb_transaction_fetch()
via 72912511.. s3:dbwrap_ctdb: add a function db_ctdb_ltdb_fetch()
via 30e493a... s3:dbrwap_ctdb: add a function db_ctdb_ltdb_store()
via 6b5f30d... s3:dbwrap_ctdb: reformat a comment slightly to enhance clearness.
from 63e324d... README.Coding: Fix typos.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-5-test
- Log -----------------------------------------------------------------
commit b58c7d96528bf3aa781f76b76465699f97e8985c
Author: Michael Adam <obnox at samba.org>
Date: Tue Nov 3 00:51:27 2009 +0100
s3:registry: add an extra check for dsize==0 to regdb_fetch_keys_internal()
Don't only rely on dptr == NULL.
I stumbled over this one when rewriting some of the dbwrap_ctdb code.
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 3fb7db84d76f15cc218554559c0f498cf00d1f7c
Author: Michael Adam <obnox at samba.org>
Date: Tue Nov 3 00:47:37 2009 +0100
s3:registry: add safety check for return value of tdb_unpack to regdb_fetch_keys_internal()
Prevents segfaults in some situations.
(For a non existent or empty record, we sometimes rely on the fetch operation
to return dsize==0 and sometimes we rely on dptr==NULL.)
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit bb939dff52c6a032719c1a2e1e4f6c056e9a0466
Author: Michael Adam <obnox at samba.org>
Date: Sat Oct 31 13:16:34 2009 +0100
s3:dbwrap_ctdb: add debug message to transaction_fetch_start()
for the case that another local process has started a transaction
bewteen releasing the transaction_lock record and starting the
transaction.
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 8b7189f4436ff9fc2bb91498ec76030cda730312
Author: Michael Adam <obnox at samba.org>
Date: Sat Oct 31 13:13:04 2009 +0100
s3:dbwrap_ctdb: split combined check in two and add descriptive debug
in db_ctdb_transaction_fetch_start() for error conditions when re-fetching
the transaction_lock record inside the transaction
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 056c2376ccfdff724701edee6b319a7d6d97822e
Author: Michael Adam <obnox at samba.org>
Date: Thu Oct 29 00:01:45 2009 +0100
s3:dbwrap_ctdb: fix race condition with concurrent transactions on the same node.
In ctdb_transaction_commit(), when the trans2_commit control fails, there
is a race condition in the 1 second sleep between the local transaction_cancel
and the call to ctdb_replay_transaction(): The database is not locked, and
neither is the transaction_lock record. So another client can start and possibly
complete a new transaction in this gap, but only on the same node: The locking
of the transaction_lock record on a different node which involves migration of
the record to the other node has been disabled by introduction of the
transaction_active flag on the db which closes precisely this gap from the start
of the commit until the call to TRANS2_FINISH or TRANS2_ERROR.
But this mechanism does not cover the case where a process on the same node
tries to start a transaction: There is no obstacle to locking the transaction_lock
record because the record does not need to be migrated.
This commit closes this race condition in ctdb_transaction_fetch_start()
by using the new ctdb_ctrl_transaction_active() call to ask the local
ctdb daemon whether it has a transaction running on the database.
If so, the check is repeated until the running transaction is done.
This does introduce an additional call to the local ctdbd when starting
transactions, but it does close the (hopefully) last race condition.
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 8296188b78c2246cd7fc2bd74fd271696aeceb6c
Author: Michael Adam <obnox at samba.org>
Date: Wed Oct 28 23:56:59 2009 +0100
s3:configure: add a check for the new CTDB_CONTROL_TRANS2_ACTIVE
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit ef3ab7ba1c513860f1698143ae1f42859fb37e56
Author: Michael Adam <obnox at samba.org>
Date: Wed Oct 28 23:56:03 2009 +0100
s3:dbwrap_ctdb: add new db_ctdb_transaction_active() that calls CTDB_CONTROL_TRANS2_COMMIT
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 7b37b8a5dcc08817b21e12570da6632674cc7575
Author: Michael Adam <obnox at samba.org>
Date: Wed Oct 28 01:54:04 2009 +0100
s3:dbwrap_ctdb: fix a race in starting concurrent transactions on a single node
There are two races in concurrent transactions on a single node.
One in starting a transaction and one with replay during commit.
This commit closes the first race by storing the client pid in the
transaction-lock record and comparing the stored pid against its own
pid after releasing the lock and refetching the record inside the
transaction.
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit f50ef43e69b1952ab182d2bb2e820680e5bb3643
Author: Michael Adam <obnox at samba.org>
Date: Wed Oct 28 01:50:15 2009 +0100
s3:dbwrap_ctdb: use db_ctdb_ltdb_fetch() inside db_ctdb_transaction_fetch_start
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 6f004b2e193208598d78599057b0d152d9aa6049
Author: Michael Adam <obnox at samba.org>
Date: Wed Oct 28 01:28:38 2009 +0100
s3:dbwrap_ctdb: use db_ctdb_ltdb_fetch() inside db_ctdb_transaction_fetch()
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 72912511a14c3c727c24c0bae06f9cece859cdbb
Author: Michael Adam <obnox at samba.org>
Date: Tue Nov 3 00:55:41 2009 +0100
s3:dbwrap_ctdb: add a function db_ctdb_ltdb_fetch()
This fetches a record from the db and splits out the ctdb header.
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 30e493a5fb411c50750d5f4263273425b9d6f14a
Author: Michael Adam <obnox at samba.org>
Date: Thu Oct 22 16:27:45 2009 +0200
s3:dbrwap_ctdb: add a function db_ctdb_ltdb_store()
and use it in db_ctdb_store() and db_ctdb_transaction_store().
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
commit 6b5f30d420fdf867443ee3aeed999feb44f6eeda
Author: Michael Adam <obnox at samba.org>
Date: Thu Oct 22 14:37:51 2009 +0200
s3:dbwrap_ctdb: reformat a comment slightly to enhance clearness.
Michael
Signed-off-by: Michael Adam <obnox at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/configure.in | 17 +++
source3/lib/dbwrap_ctdb.c | 234 ++++++++++++++++++++++++++++--------
source3/registry/reg_backend_db.c | 6 +-
3 files changed, 204 insertions(+), 53 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/configure.in b/source3/configure.in
index 567e692..e42ca3e 100644
--- a/source3/configure.in
+++ b/source3/configure.in
@@ -5154,6 +5154,23 @@ else
ctdb_broken="missing transaction support"
fi
+AC_HAVE_DECL(CTDB_CONTROL_TRANS2_ACTIVE,[
+#include "confdefs.h"
+#define NO_CONFIG_H
+#include "replace.h"
+#include "system/wait.h"
+#include "system/network.h"
+#include <talloc.h>
+#include <tdb.h>
+#include <ctdb.h>
+#include <ctdb_private.h>
+])
+if test x"$ac_cv_have_CTDB_CONTROL_TRANS2_ACTIVE_decl" = x"yes"; then
+ ctdb_broken=no
+else
+ ctdb_broken="transaction support too old"
+fi
+
# in ctdb 1.0.57 ctdb_control_tcp was temparary renamed to ctdb_tcp_client
AC_CHECK_TYPE(struct ctdb_tcp_client,[
AC_DEFINE([ctdb_control_tcp],[ctdb_tcp_client],[ctdb ipv4 support])
diff --git a/source3/lib/dbwrap_ctdb.c b/source3/lib/dbwrap_ctdb.c
index 07dde1e..8563990 100644
--- a/source3/lib/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap_ctdb.c
@@ -26,9 +26,11 @@
struct db_ctdb_transaction_handle {
struct db_ctdb_ctx *ctx;
bool in_replay;
- /* we store the reads and writes done under a transaction one
- list stores both reads and writes, the other just writes
- */
+ /*
+ * we store the reads and writes done under a transaction:
+ * - one list stores both reads and writes (m_all),
+ * - the other just writes (m_write)
+ */
struct ctdb_marshall_buffer *m_all;
struct ctdb_marshall_buffer *m_write;
uint32_t nesting;
@@ -73,6 +75,91 @@ static NTSTATUS tdb_error_to_ntstatus(struct tdb_context *tdb)
}
+/**
+ * fetch a record from the tdb, separating out the header
+ * information and returning the body of the record.
+ */
+static NTSTATUS db_ctdb_ltdb_fetch(struct db_ctdb_ctx *db,
+ TDB_DATA key,
+ struct ctdb_ltdb_header *header,
+ TALLOC_CTX *mem_ctx,
+ TDB_DATA *data)
+{
+ TDB_DATA rec;
+ NTSTATUS status;
+
+ rec = tdb_fetch(db->wtdb->tdb, key);
+ if (rec.dsize < sizeof(struct ctdb_ltdb_header)) {
+ status = NT_STATUS_NOT_FOUND;
+ if (data) {
+ ZERO_STRUCTP(data);
+ }
+ if (header) {
+ header->dmaster = (uint32_t)-1;
+ header->rsn = 0;
+ }
+ goto done;
+ }
+
+ if (header) {
+ *header = *(struct ctdb_ltdb_header *)rec.dptr;
+ }
+
+ if (data) {
+ data->dsize = rec.dsize - sizeof(struct ctdb_ltdb_header);
+ if (data->dsize == 0) {
+ data->dptr = NULL;
+ } else {
+ data->dptr = (unsigned char *)talloc_memdup(mem_ctx,
+ rec.dptr
+ + sizeof(struct ctdb_ltdb_header),
+ data->dsize);
+ if (data->dptr == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto done;
+ }
+ }
+ }
+
+ status = NT_STATUS_OK;
+
+done:
+ SAFE_FREE(rec.dptr);
+ return status;
+}
+
+/*
+ * Store a record together with the ctdb record header
+ * in the local copy of the database.
+ */
+static NTSTATUS db_ctdb_ltdb_store(struct db_ctdb_ctx *db,
+ TDB_DATA key,
+ struct ctdb_ltdb_header *header,
+ TDB_DATA data)
+{
+ TALLOC_CTX *tmp_ctx = talloc_stackframe();
+ TDB_DATA rec;
+ int ret;
+
+ rec.dsize = data.dsize + sizeof(struct ctdb_ltdb_header);
+ rec.dptr = (uint8_t *)talloc_size(tmp_ctx, rec.dsize);
+
+ if (rec.dptr == NULL) {
+ talloc_free(tmp_ctx);
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ memcpy(rec.dptr, header, sizeof(struct ctdb_ltdb_header));
+ memcpy(sizeof(struct ctdb_ltdb_header) + (uint8_t *)rec.dptr, data.dptr, data.dsize);
+
+ ret = tdb_store(db->wtdb->tdb, key, rec, TDB_REPLACE);
+
+ talloc_free(tmp_ctx);
+
+ return (ret == 0) ? NT_STATUS_OK
+ : tdb_error_to_ntstatus(db->wtdb->tdb);
+
+}
/*
form a ctdb_rec_data record from a key/data pair
@@ -211,6 +298,27 @@ static struct ctdb_rec_data *db_ctdb_marshall_loop_next(struct ctdb_marshall_buf
}
+static int32_t db_ctdb_transaction_active(uint32_t db_id)
+{
+ int32_t status;
+ NTSTATUS ret;
+ TDB_DATA indata;
+
+ indata.dptr = (uint8_t *)&db_id;
+ indata.dsize = sizeof(db_id);
+
+ ret = ctdbd_control_local(messaging_ctdbd_connection(),
+ CTDB_CONTROL_TRANS2_ACTIVE, 0, 0,
+ indata, NULL, NULL, &status);
+
+ if (!NT_STATUS_IS_OK(ret)) {
+ DEBUG(2, ("ctdb control TRANS2_ACTIVE failed\n"));
+ return -1;
+ }
+
+ return status;
+}
+
/**
* CTDB transaction destructor
@@ -229,12 +337,17 @@ static int db_ctdb_transaction_destructor(struct db_ctdb_transaction_handle *h)
static int db_ctdb_transaction_fetch_start(struct db_ctdb_transaction_handle *h)
{
struct db_record *rh;
+ struct db_ctdb_rec *crec;
TDB_DATA key;
TALLOC_CTX *tmp_ctx;
const char *keyname = CTDB_TRANSACTION_LOCK_KEY;
int ret;
struct db_ctdb_ctx *ctx = h->ctx;
TDB_DATA data;
+ pid_t pid;
+ NTSTATUS status;
+ struct ctdb_ltdb_header header;
+ int32_t transaction_status;
key.dptr = (uint8_t *)discard_const(keyname);
key.dsize = strlen(keyname);
@@ -248,6 +361,34 @@ again:
talloc_free(tmp_ctx);
return -1;
}
+ crec = talloc_get_type_abort(rh->private_data, struct db_ctdb_rec);
+
+ transaction_status = db_ctdb_transaction_active(ctx->db_id);
+ if (transaction_status == 1) {
+ unsigned long int usec = (1000 + random()) % 100000;
+ DEBUG(3, ("Transaction already active on db_id[0x%08x]."
+ "Re-trying after %lu microseconds...",
+ ctx->db_id, usec));
+ talloc_free(tmp_ctx);
+ usleep(usec);
+ goto again;
+ }
+
+ /*
+ * store the pid in the database:
+ * it is not enought that the node is dmaster...
+ */
+ pid = getpid();
+ data.dptr = (unsigned char *)&pid;
+ data.dsize = sizeof(pid_t);
+ status = db_ctdb_ltdb_store(ctx, key, &(crec->header), data);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(0, (__location__ " Failed to store pid in transaction "
+ "record: %s\n", nt_errstr(status)));
+ talloc_free(tmp_ctx);
+ return -1;
+ }
+
talloc_free(rh);
ret = tdb_transaction_start(ctx->wtdb->tdb);
@@ -257,17 +398,36 @@ again:
return -1;
}
- data = tdb_fetch(ctx->wtdb->tdb, key);
- if ((data.dptr == NULL) ||
- (data.dsize < sizeof(struct ctdb_ltdb_header)) ||
- ((struct ctdb_ltdb_header *)data.dptr)->dmaster != get_my_vnn()) {
- SAFE_FREE(data.dptr);
+ status = db_ctdb_ltdb_fetch(ctx, key, &header, tmp_ctx, &data);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(0, (__location__ " failed to refetch transaction lock "
+ "record inside transaction: %s - retrying\n",
+ nt_errstr(status)));
+ tdb_transaction_cancel(ctx->wtdb->tdb);
+ talloc_free(tmp_ctx);
+ goto again;
+ }
+
+ if (header.dmaster != get_my_vnn()) {
+ DEBUG(3, (__location__ " refetch transaction lock record : "
+ "we are not dmaster any more "
+ "(dmaster[%u] != my_vnn[%u]) - retrying\n",
+ header.dmaster, get_my_vnn()));
+ tdb_transaction_cancel(ctx->wtdb->tdb);
+ talloc_free(tmp_ctx);
+ goto again;
+ }
+
+ if ((data.dsize != sizeof(pid_t)) || (*(pid_t *)(data.dptr) != pid)) {
+ DEBUG(3, (__location__ " refetch transaction lock record: "
+ "another local process has started a transaction "
+ "(stored pid [%u] != my pid [%u]) - retrying\n",
+ *(pid_t *)(data.dptr), pid));
tdb_transaction_cancel(ctx->wtdb->tdb);
talloc_free(tmp_ctx);
goto again;
}
- SAFE_FREE(data.dptr);
talloc_free(tmp_ctx);
return 0;
@@ -329,24 +489,14 @@ static int db_ctdb_transaction_fetch(struct db_ctdb_ctx *db,
TDB_DATA key, TDB_DATA *data)
{
struct db_ctdb_transaction_handle *h = db->transaction;
+ NTSTATUS status;
- *data = tdb_fetch(h->ctx->wtdb->tdb, key);
+ status = db_ctdb_ltdb_fetch(h->ctx, key, NULL, mem_ctx, data);
- if (data->dptr != NULL) {
- uint8_t *oldptr = (uint8_t *)data->dptr;
- data->dsize -= sizeof(struct ctdb_ltdb_header);
- if (data->dsize == 0) {
- data->dptr = NULL;
- } else {
- data->dptr = (uint8 *)
- talloc_memdup(
- mem_ctx, data->dptr+sizeof(struct ctdb_ltdb_header),
- data->dsize);
- }
- SAFE_FREE(oldptr);
- if (data->dptr == NULL && data->dsize != 0) {
- return -1;
- }
+ if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+ *data = tdb_null;
+ } else if (!NT_STATUS_IS_OK(status)) {
+ return -1;
}
if (!h->in_replay) {
@@ -470,6 +620,7 @@ static int db_ctdb_transaction_store(struct db_ctdb_transaction_handle *h,
int ret;
TDB_DATA rec;
struct ctdb_ltdb_header header;
+ NTSTATUS status;
/* we need the header so we can update the RSN */
rec = tdb_fetch(h->ctx->wtdb->tdb, key);
@@ -510,17 +661,12 @@ static int db_ctdb_transaction_store(struct db_ctdb_transaction_handle *h,
return -1;
}
- rec.dsize = data.dsize + sizeof(struct ctdb_ltdb_header);
- rec.dptr = (uint8_t *)talloc_size(tmp_ctx, rec.dsize);
- if (rec.dptr == NULL) {
- DEBUG(0,(__location__ " Failed to alloc record\n"));
- talloc_free(tmp_ctx);
- return -1;
+ status = db_ctdb_ltdb_store(h->ctx, key, &header, data);
+ if (NT_STATUS_IS_OK(status)) {
+ ret = 0;
+ } else {
+ ret = -1;
}
- memcpy(rec.dptr, &header, sizeof(struct ctdb_ltdb_header));
- memcpy(sizeof(struct ctdb_ltdb_header) + (uint8_t *)rec.dptr, data.dptr, data.dsize);
-
- ret = tdb_store(h->ctx->wtdb->tdb, key, rec, TDB_REPLACE);
talloc_free(tmp_ctx);
@@ -784,24 +930,8 @@ static NTSTATUS db_ctdb_store(struct db_record *rec, TDB_DATA data, int flag)
{
struct db_ctdb_rec *crec = talloc_get_type_abort(
rec->private_data, struct db_ctdb_rec);
- TDB_DATA cdata;
- int ret;
-
- cdata.dsize = sizeof(crec->header) + data.dsize;
- if (!(cdata.dptr = SMB_MALLOC_ARRAY(uint8, cdata.dsize))) {
- return NT_STATUS_NO_MEMORY;
- }
-
- memcpy(cdata.dptr, &crec->header, sizeof(crec->header));
- memcpy(cdata.dptr + sizeof(crec->header), data.dptr, data.dsize);
-
- ret = tdb_store(crec->ctdb_ctx->wtdb->tdb, rec->key, cdata, TDB_REPLACE);
-
- SAFE_FREE(cdata.dptr);
-
- return (ret == 0) ? NT_STATUS_OK
- : tdb_error_to_ntstatus(crec->ctdb_ctx->wtdb->tdb);
+ return db_ctdb_ltdb_store(crec->ctdb_ctx, rec->key, &(crec->header), data);
}
diff --git a/source3/registry/reg_backend_db.c b/source3/registry/reg_backend_db.c
index 2b6259c..43963db 100644
--- a/source3/registry/reg_backend_db.c
+++ b/source3/registry/reg_backend_db.c
@@ -1461,7 +1461,7 @@ static WERROR regdb_fetch_keys_internal(struct db_context *db, const char *key,
value = regdb_fetch_key_internal(db, frame, key);
- if (value.dptr == NULL) {
+ if (value.dsize == 0 || value.dptr == NULL) {
DEBUG(10, ("regdb_fetch_keys: no subkeys found for key [%s]\n",
key));
goto done;
@@ -1470,6 +1470,10 @@ static WERROR regdb_fetch_keys_internal(struct db_context *db, const char *key,
buf = value.dptr;
buflen = value.dsize;
len = tdb_unpack( buf, buflen, "d", &num_items);
+ if (len == (uint32_t)-1) {
+ werr = WERR_NOT_FOUND;
+ goto done;
+ }
werr = regsubkey_ctr_reinit(ctr);
W_ERROR_NOT_OK_GOTO_DONE(werr);
--
Samba Shared Repository
More information about the samba-cvs
mailing list