libctdb: change callback for ctdb_readrecordlock.
authorRusty Russell <rusty@rustcorp.com.au>
Fri, 4 Jun 2010 04:03:08 +0000 (13:33 +0930)
committerRusty Russell <rusty@rustcorp.com.au>
Fri, 4 Jun 2010 04:03:08 +0000 (13:33 +0930)
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@rustcorp.com.au>
include/ctdb.h
libctdb/ctdb.c
libctdb/tst.c

index 32a7a2c357001c8e7e526664eaecac994533d683..fa3f30dc3fd3ce1e707ea97c0283de7927428212 100644 (file)
@@ -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),       \
index 43c2829c5724038a95a4ab43ab62f1cf447fc025..76e8d50a28168f9ed978468fe45dbc4e2f58a2aa 100644 (file)
@@ -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)
@@ -527,10 +527,9 @@ 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)
@@ -543,7 +542,7 @@ void ctdb_release_lock(struct ctdb_lock *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 +550,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,67 +562,52 @@ 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;
-}
-
 static void readrecordlock_retry(struct ctdb_connection *ctdb,
                                 struct ctdb_request *req, void *private)
 {
        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. */
                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);
                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 +616,23 @@ 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);
+               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);
                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 +645,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;
 }
 
index 6fd376a7f86a2b8ac8024434fbfcf6ef35585a4b..090b26da3a71a43f13c59428fe0dd4ff4a1db086 100644 (file)
@@ -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 bool rrl_cb_called;
+
+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];
 
-       lock = ctdb_readrecordlock_recv(private, req, &outdata);
+       rrl_cb_called = true;
        if (!lock) {
                printf("rrl_cb returned error\n");
                return;
@@ -194,12 +194,12 @@ int main(int argc, char *argv[])
                exit(10);
        }
 
-       if (!ctdb_readrecordlock_send(ctdb_db_context, key, &handle,
-                                     rrl_cb, ctdb_db_context)) {
+       if (!ctdb_readrecordlock_async(ctdb_db_context, key,
+                                      rrl_cb, ctdb_db_context)) {
                printf("Failed to send READRECORDLOCK\n");
                exit(10);
        }
-       if (handle) {
+       if (!rrl_cb_called) {
                printf("READRECORDLOCK is async\n");
        }
        for (;;) {