[SCM] CTDB repository - branch master updated - ctdb-1.0.114-129-g87dc18a
Ronnie Sahlberg
sahlberg at samba.org
Thu Jun 3 22:51:38 MDT 2010
The branch, master has been updated
via 87dc18a3a051da04685f14529c53c428d37c2912 (commit)
via 0ba458a91ba510215a9c8770286a68e19911515f (commit)
via 03b5546ae45a60ab41eb4f7159a45bfdbf959888 (commit)
via 8626b6d7d4e5c747b6bac9d5e5b2cd302b6e144c (commit)
from d42ea3b1892f6a4abd1dbcf822d0a4d5db422d38 (commit)
http://gitweb.samba.org/?p=sahlberg/ctdb.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 87dc18a3a051da04685f14529c53c428d37c2912
Author: Ronnie Sahlberg <ronniesahlberg at gmail.com>
Date: Fri Jun 4 14:47:06 2010 +1000
Readrecordlock changes:
Make the use of ctdb_release_lock() mandatory from the callback.
Split ctdb_release_lock() in two, release the tdb lock in the
ctdb_release_lock() function and move the freeing of the lock structure to ctdb_free_lock() which is private to libctdb.
When the callback returns, verify that the callback has actually released the lock and warn (FIXME) if not.
Update ctdb_writerecord to warn and fail (FIXME) if writing while the lock is not held.
commit 0ba458a91ba510215a9c8770286a68e19911515f
Author: Ronnie Sahlberg <ronniesahlberg at gmail.com>
Date: Fri Jun 4 14:20:17 2010 +1000
remove the global rrl_cb_called from the libctdb example
and psss it through the callback via private_data.
add a comment that the callback may sometimes have already been invoked
when the ctdb_readrecordlock_async() call returns
and that the application can use *private_data IF the application
needs to know if the callback has already triggered or not.
commit 03b5546ae45a60ab41eb4f7159a45bfdbf959888
Author: Rusty Russell <rusty at rustcorp.com.au>
Date: Fri Jun 4 13:33:08 2010 +0930
libctdb: change callback for ctdb_readrecordlock.
After discussion with Ronnie, we decided to revisit this interface. We use
the name ctdb_readrecordlock_async, as it is *not* always a send, and we
use a specific callback to avoid the "fake request" creation on the fast
path.
The request itself is never exposed: this means it can't be cancelled,
but we can revisit that later if need be.
This makes both use and implementation simpler.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
commit 8626b6d7d4e5c747b6bac9d5e5b2cd302b6e144c
Author: Rusty Russell <rusty at rustcorp.com.au>
Date: Fri Jun 4 13:34:06 2010 +0930
libctdb: fix wrong argument being handed to callback on attachdb fail
When attachdb failed, we were handing the db, not the user-supplied
arg to the callback.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
-----------------------------------------------------------------------
Summary of changes:
include/ctdb.h | 28 ++++++++--------
libctdb/ctdb.c | 95 ++++++++++++++++++++++++++++---------------------------
libctdb/tst.c | 27 +++++++++++-----
3 files changed, 81 insertions(+), 69 deletions(-)
Changeset truncated at 500 lines:
diff --git a/include/ctdb.h b/include/ctdb.h
index 32a7a2c..fa3f30d 100644
--- a/include/ctdb.h
+++ b/include/ctdb.h
@@ -100,16 +100,16 @@ struct ctdb_lock;
* When the lock is released, data is freed too, so make sure to copy the data
* before that.
*
- * This returns true on success, and req will be non-NULL if a request was
- * actually sent, otherwise callback will have already been called.
+ * This returns true on success: the callback may have already been called,
+ * or it might be awaiting a response from ctdbd.
*/
+typedef void (*ctdb_rrl_callback_t)(struct ctdb_db *ctdb_db,
+ struct ctdb_lock *lock,
+ TDB_DATA data,
+ void *private);
bool
-ctdb_readrecordlock_send(struct ctdb_db *ctdb_db, TDB_DATA key,
- struct ctdb_request **req,
- ctdb_callback_t callback, void *private_data);
-struct ctdb_lock *ctdb_readrecordlock_recv(struct ctdb_db *ctdb_db,
- struct ctdb_request *handle,
- TDB_DATA *data);
+ctdb_readrecordlock_async(struct ctdb_db *ctdb_db, TDB_DATA key,
+ ctdb_rrl_callback_t callback, void *private_data);
/* Returns null on failure. */
struct ctdb_lock *ctdb_readrecordlock(struct ctdb_db *ctdb_db, TDB_DATA key,
@@ -118,9 +118,7 @@ struct ctdb_lock *ctdb_readrecordlock(struct ctdb_db *ctdb_db, TDB_DATA key,
/*
* Function to write data to a record
* This function may ONLY be called while holding a lock to the record
- * created by ctdb_readrecordlock*
- * Either from the callback provided to ctdb_readrecordlock_send()
- * or after calling ctdb_readrecordlock_recv() but before calling
+ * created by ctdb_readrecordlock*, and before calling
* ctdb_release_lock() to release the lock.
*/
int ctdb_writerecord(struct ctdb_lock *lock, TDB_DATA data);
@@ -221,9 +219,11 @@ int ctdb_cancel(struct ctdb_request *);
ctdb_attachdb_send((ctdb), (name), (persistent), (tdb_flags), \
ctdb_sendcb((cb), (cbdata)), (cbdata))
-#define ctdb_readrecordlock_send(ctdb_db, key, reqp, cb, cbdata) \
- ctdb_readrecordlock_send((ctdb_db), (key), (reqp), \
- ctdb_sendcb((cb), (cbdata)), (cbdata))
+#define ctdb_readrecordlock_async(_ctdb_db, key, cb, cbdata) \
+ ctdb_readrecordlock_async((_ctdb_db), (key), \
+ typesafe_cb_preargs(void, (cb), (cbdata), \
+ struct ctdb_db *, struct ctdb_lock *, \
+ TDB_DATA), (cbdata))
#define ctdb_set_message_handler_send(ctdb, srvid, handler, cb, cbdata) \
ctdb_set_message_handler_send((ctdb), (srvid), (handler), \
diff --git a/libctdb/ctdb.c b/libctdb/ctdb.c
index 8700a60..52cd38e 100644
--- a/libctdb/ctdb.c
+++ b/libctdb/ctdb.c
@@ -33,7 +33,7 @@
/* Remove type-safety macros. */
#undef ctdb_attachdb_send
-#undef ctdb_readrecordlock_send
+#undef ctdb_readrecordlock_async
/* FIXME: Could be in shared util code with rest of ctdb */
static void close_noerr(int fd)
@@ -449,7 +449,7 @@ static void attachdb_done(struct ctdb_connection *ctdb,
if (!reply || reply->status != 0) {
/* We failed. Hand request to user and have them discover it
* via ctdb_attachdb_recv. */
- db->callback(ctdb, req, db);
+ db->callback(ctdb, req, db->private_data);
return;
}
db->id = *(uint32_t *)reply->data;
@@ -460,7 +460,7 @@ static void attachdb_done(struct ctdb_connection *ctdb,
&db->id, sizeof(db->id),
attachdb_getdbpath_done, db);
if (!req2) {
- db->callback(ctdb, req, db);
+ db->callback(ctdb, req, db->private_data);
return;
}
req->extra = req2;
@@ -527,23 +527,32 @@ struct ctdb_lock {
/* This will always be true by the time user sees this. */
bool held;
struct ctdb_ltdb_header *hdr;
- TDB_DATA data;
/* For convenience, we stash original callback here. */
- ctdb_callback_t callback;
+ ctdb_rrl_callback_t callback;
};
void ctdb_release_lock(struct ctdb_lock *lock)
{
if (lock->held) {
tdb_chainunlock(lock->ctdb_db->tdb, lock->key);
+ lock->held = false;
}
- free(lock->hdr); /* Also frees data */
+}
+
+static void ctdb_free_lock(struct ctdb_lock *lock)
+{
+ if (lock->held) {
+ /* FIXME: report error. Callback never released the lock */
+ ctdb_release_lock(lock);
+ }
+
+ free(lock->hdr);
free(lock);
}
/* We keep the lock if local node is the dmaster. */
-static bool try_readrecordlock(struct ctdb_lock *lock)
+static bool try_readrecordlock(struct ctdb_lock *lock, TDB_DATA *data)
{
struct ctdb_ltdb_header *hdr;
@@ -551,7 +560,7 @@ static bool try_readrecordlock(struct ctdb_lock *lock)
return NULL;
}
- hdr = ctdb_local_fetch(lock->ctdb_db->tdb, lock->key, &lock->data);
+ hdr = ctdb_local_fetch(lock->ctdb_db->tdb, lock->key, data);
if (hdr && hdr->dmaster == lock->ctdb_db->ctdb->pnn) {
lock->held = true;
lock->hdr = hdr;
@@ -563,28 +572,11 @@ static bool try_readrecordlock(struct ctdb_lock *lock)
return NULL;
}
-/* If they cancel *before* we hand them the lock from
- * ctdb_readrecordlock_recv, we free it here. */
+/* If they shutdown before we hand them the lock, we free it here. */
static void destroy_lock(struct ctdb_request *req)
{
ctdb_release_lock(req->extra);
-}
-
-struct ctdb_lock *ctdb_readrecordlock_recv(struct ctdb_db *ctdb_db,
- struct ctdb_request *req,
- TDB_DATA *data)
-{
- struct ctdb_lock *lock = req->extra;
-
- if (!lock->held) {
- /* Something went wrong. */
- return NULL;
- }
-
- /* Now it's their responsibility to free! */
- req->extra_destructor = NULL;
- *data = lock->data;
- return lock;
+ ctdb_free_lock(req->extra);
}
static void readrecordlock_retry(struct ctdb_connection *ctdb,
@@ -592,38 +584,43 @@ static void readrecordlock_retry(struct ctdb_connection *ctdb,
{
struct ctdb_lock *lock = req->extra;
struct ctdb_reply_call *reply;
+ TDB_DATA data;
/* OK, we've received reply to noop migration */
reply = unpack_reply_call(req, CTDB_NULL_FUNC);
if (!reply || reply->status != 0) {
- lock->callback(ctdb, req, private);
+ lock->callback(lock->ctdb_db, NULL, tdb_null, private);
+ ctdb_request_free(req); /* Also frees lock. */
+ ctdb_free_lock(lock);
return;
}
/* Can we get lock now? */
- if (try_readrecordlock(lock)) {
- lock->callback(ctdb, req, private);
+ if (try_readrecordlock(lock, &data)) {
+ /* Now it's their responsibility to free lock & request! */
+ req->extra_destructor = NULL;
+ lock->callback(lock->ctdb_db, lock, data, private);
+ ctdb_free_lock(lock);
return;
}
/* Retransmit the same request again (we lost race). */
io_elem_reset(req->io);
DLIST_ADD(ctdb->outq, req);
- return;
}
bool
-ctdb_readrecordlock_send(struct ctdb_db *ctdb_db, TDB_DATA key,
- struct ctdb_request **reqp,
- ctdb_callback_t callback, void *cbdata)
+ctdb_readrecordlock_async(struct ctdb_db *ctdb_db, TDB_DATA key,
+ ctdb_rrl_callback_t callback, void *cbdata)
{
struct ctdb_request *req;
struct ctdb_lock *lock;
+ TDB_DATA data;
- /* Setup lock. */
+ /* Setup lock */
lock = malloc(sizeof(*lock) + key.dsize);
if (!lock) {
- return NULL;
+ return false;
}
lock->key.dptr = (void *)(lock + 1);
memcpy(lock->key.dptr, key.dptr, key.dsize);
@@ -632,25 +629,25 @@ ctdb_readrecordlock_send(struct ctdb_db *ctdb_db, TDB_DATA key,
lock->hdr = NULL;
lock->held = false;
- /* Get ready in case we need to send a migrate request. */
+ /* Fast path. */
+ if (try_readrecordlock(lock, &data)) {
+ callback(ctdb_db, lock, data, cbdata);
+ ctdb_free_lock(lock);
+ return true;
+ }
+
+ /* Slow path: create request. */
req = new_ctdb_request(offsetof(struct ctdb_req_call, data)
- + key.dsize, callback, cbdata);
+ + key.dsize, readrecordlock_retry, cbdata);
if (!req) {
ctdb_release_lock(lock);
+ ctdb_free_lock(lock);
return NULL;
}
req->extra = lock;
req->extra_destructor = destroy_lock;
-
- if (try_readrecordlock(lock)) {
- *reqp = NULL;
- callback(ctdb_db->ctdb, req, cbdata);
- return true;
- }
-
/* We store the original callback in the lock, and use our own. */
lock->callback = callback;
- req->callback = readrecordlock_retry;
io_elem_init_req_header(req->io, CTDB_REQ_CALL, CTDB_CURRENT_NODE,
new_reqid(ctdb_db->ctdb));
@@ -663,7 +660,6 @@ ctdb_readrecordlock_send(struct ctdb_db *ctdb_db, TDB_DATA key,
req->hdr.call->calldatalen = 0;
memcpy(req->hdr.call->data, key.dptr, key.dsize);
DLIST_ADD(ctdb_db->ctdb->outq, req);
- *reqp = req;
return true;
}
@@ -674,6 +670,11 @@ int ctdb_writerecord(struct ctdb_lock *lock, TDB_DATA data)
return -1;
}
+ if (!lock->held) {
+ /* FIXME: Report error. */
+ return -1;
+ }
+
return ctdb_local_store(lock->ctdb_db->tdb, lock->key, lock->hdr,
data);
}
diff --git a/libctdb/tst.c b/libctdb/tst.c
index 6fd376a..f1a8e3d 100644
--- a/libctdb/tst.c
+++ b/libctdb/tst.c
@@ -81,15 +81,15 @@ static void rm_cb(struct ctdb_connection *ctdb,
*
* Pure read, or pure write are just special cases of this cycle.
*/
-static void rrl_cb(struct ctdb_connection *ctdb,
- struct ctdb_request *req, void *private)
+static void rrl_cb(struct ctdb_db *ctdb_db,
+ struct ctdb_lock *lock, TDB_DATA outdata, void *private)
{
- struct ctdb_lock *lock;
- TDB_DATA outdata;
TDB_DATA data;
char tmp[256];
+ bool *rrl_cb_called = private;
+
+ *rrl_cb_called = true;
- lock = ctdb_readrecordlock_recv(private, req, &outdata);
if (!lock) {
printf("rrl_cb returned error\n");
return;
@@ -134,6 +134,8 @@ int main(int argc, char *argv[])
uint32_t recmaster;
int ret;
TDB_DATA msg;
+ bool rrl_cb_called = false;
+
ctdb_connection = ctdb_connect("/tmp/ctdb.socket");
if (!ctdb_connection)
@@ -194,12 +196,21 @@ int main(int argc, char *argv[])
exit(10);
}
- if (!ctdb_readrecordlock_send(ctdb_db_context, key, &handle,
- rrl_cb, ctdb_db_context)) {
+ /* In the non-contended case the callback might be invoked
+ * immediately, before ctdb_readrecordlock_async() returns.
+ * In the contended case the callback will be invoked later.
+ *
+ * Normally an application would not care whether the callback
+ * has already been invoked here or not, but if the application
+ * needs to know, it can use the *private_data pointer
+ * to pass data through to the callback and back.
+ */
+ if (!ctdb_readrecordlock_async(ctdb_db_context, key,
+ rrl_cb, &rrl_cb_called)) {
printf("Failed to send READRECORDLOCK\n");
exit(10);
}
- if (handle) {
+ if (!rrl_cb_called) {
printf("READRECORDLOCK is async\n");
}
for (;;) {
--
CTDB repository
More information about the samba-cvs
mailing list