Make fetch_locked more scalable
authorVolker Lendecke <vl@samba.org>
Wed, 9 Dec 2009 14:11:45 +0000 (15:11 +0100)
committerMichael Adam <obnox@samba.org>
Fri, 11 Dec 2009 23:45:39 +0000 (00:45 +0100)
This patch improves the handling of the fetch_lock operation on non-persistent
databases that ctdb clients have to do very frequently.

The normal flow how this goes is the following:

1. Client does a local fetch_lock on the database

2. Client looks if the local node is dmaster.
   If yes, everything is fine
   If no, continue here

3. Client unlocks the local record

4. Client issues a "get me the record" call to ctdbd

5. ctdbd goes out and fetches the dmaster role

6. ctdbd tells the client to retry

7. Client starts over again

The problem is between step 6 and 7: Before the client has had the chance to
retry (i.e. catch the record with a fetch_locked), another node might have come
asking ctdbd to migrate away the record again. This is a real problem, I've
seen >20 loops of this kind in real workloads.

This patch does the following: Whenever ctdb receives a record as result of
step 5, it puts the key on a "holdback list". As long as a key is on this list,
a request to migrate away the dmaster is put on hold. It is the client's duty
to issue the "CTDB_CONTROL_GOTIT" control when it has successfully done step 2
after having asked ctdb to fetch the record. This will release the key from the
"holdback list" and re-issue all dmaster migration requests.

As a safeguard against malicious clients, once a second (default 1000msecs,
tunable "HoldbackCleanupInterval" in milliseconds) ctdbd goes over the list of
held back keys, deletes them and releases all held back migration requests.

(This used to be ctdb commit 5736e17c139c9a8049e235429aeae0c6c9d0e93d)

ctdb/include/ctdb_private.h
ctdb/server/ctdb_call.c
ctdb/server/ctdb_control.c
ctdb/server/ctdb_daemon.c
ctdb/server/ctdb_recover.c
ctdb/server/ctdb_recoverd.c
ctdb/server/ctdb_tunables.c

index e4f4aba091497cf7f54dab5f343f93aa44f00080..041c0e3f89af9d3cc06ec70038dd68434a56af17 100644 (file)
@@ -95,6 +95,7 @@ struct ctdb_tunable {
        uint32_t traverse_timeout;
        uint32_t keepalive_interval;
        uint32_t keepalive_limit;
+       uint32_t holdback_cleanup_interval;
        uint32_t max_lacount;
        uint32_t recover_timeout;
        uint32_t recover_interval;
@@ -397,6 +398,7 @@ struct ctdb_context {
        uint32_t recovery_mode;
        TALLOC_CTX *tickle_update_context;
        TALLOC_CTX *keepalive_ctx;
+       struct timed_event *holdback_cleanup_te;
        struct ctdb_tunable tunable;
        enum ctdb_freeze_mode freeze_mode[NUM_DB_PRIORITIES+1];
        struct ctdb_freeze_handle *freeze_handles[NUM_DB_PRIORITIES+1];
@@ -478,6 +480,17 @@ struct ctdb_db_context {
        struct ctdb_traverse_local_handle *traverse;
        bool transaction_active;
        struct ctdb_vacuum_handle *vacuum_handle;
+
+       /*
+        * The keys to hold back until CTDB_CONTROL_GOTIT is being
+        * sent by a client having forced a migration to us.
+        */
+       uint8_t **holdback_keys;
+
+       /*
+        * The CTDB_REQ_CALLs held back according to "holdback_keys"
+        */
+       struct ctdb_req_header **held_back;
 };
 
 
@@ -627,6 +640,7 @@ enum ctdb_controls {CTDB_CONTROL_PROCESS_EXISTS          = 0,
                    CTDB_CONTROL_CLEAR_LOG               = 118,
                    CTDB_CONTROL_TRANS3_COMMIT           = 119,
                    CTDB_CONTROL_GET_DB_SEQNUM           = 120,
+                   CTDB_CONTROL_GOTIT                   = 121,
 };     
 
 /*
@@ -1170,6 +1184,11 @@ struct ctdb_control_wipe_database {
        uint32_t transaction_id;
 };
 
+struct ctdb_control_gotit {
+       uint32_t db_id;
+       uint8_t key[1];
+};
+
 /*
   state of a in-progress ctdb call in client
 */
@@ -1238,6 +1257,10 @@ void ctdb_start_keepalive(struct ctdb_context *ctdb);
 void ctdb_stop_keepalive(struct ctdb_context *ctdb);
 int32_t ctdb_run_eventscripts(struct ctdb_context *ctdb, struct ctdb_req_control *c, TDB_DATA data, bool *async_reply);
 
+void ctdb_start_holdback_cleanup(struct ctdb_context *ctdb);
+void ctdb_stop_holdback_cleanup(struct ctdb_context *ctdb);
+int32_t ctdb_control_gotit(struct ctdb_context *ctdb, TDB_DATA indata);
+
 
 void ctdb_daemon_cancel_controls(struct ctdb_context *ctdb, struct ctdb_node *node);
 void ctdb_call_resend_all(struct ctdb_context *ctdb);
index 82c1304e07591ff7eb7a7c645bed01d500254142..443e6d2d3e832f74a10bb9af2459eedd7b214436 100644 (file)
@@ -231,6 +231,30 @@ static void ctdb_call_send_dmaster(struct ctdb_db_context *ctdb_db,
        talloc_free(r);
 }
 
+static void ctdb_hold_back_key(struct ctdb_db_context *db, TDB_DATA key)
+{
+       size_t num_keys;
+       uint8_t **tmp;
+
+       DEBUG(DEBUG_INFO, ("Holding back key %08x\n", ctdb_hash(&key)));
+
+       num_keys = talloc_array_length(db->holdback_keys);
+       tmp = talloc_realloc(db, db->holdback_keys, uint8_t *, num_keys+1);
+       if (tmp == NULL) {
+               DEBUG(DEBUG_ERR, ("talloc_realloc failed\n"));
+               return;
+       }
+       db->holdback_keys = tmp;
+
+       db->holdback_keys[num_keys] = (uint8_t *)talloc_memdup(
+               db->holdback_keys, key.dptr, key.dsize);
+       if (db->holdback_keys[num_keys] == NULL) {
+               DEBUG(DEBUG_ERR, ("talloc_memdup failed\n"));
+               db->holdback_keys = talloc_realloc(db, db->holdback_keys,
+                                                  uint8_t *, num_keys);
+       }
+}
+
 /*
   called when a CTDB_REPLY_DMASTER packet comes in, or when the lmaster
   gets a CTDB_REQUEST_DMASTER for itself. We become the dmaster.
@@ -274,6 +298,8 @@ static void ctdb_become_dmaster(struct ctdb_db_context *ctdb_db,
                return;
        }
 
+       ctdb_hold_back_key(ctdb_db, key);
+
        ctdb_call_local(ctdb_db, state->call, &header, state, &data, ctdb->pnn);
 
        ctdb_ltdb_unlock(ctdb_db, state->call->key);
@@ -368,6 +394,47 @@ void ctdb_request_dmaster(struct ctdb_context *ctdb, struct ctdb_req_header *hdr
        }
 }
 
+/*
+ * Did we just pull the dmaster of the record for a client fetch_lock,
+ * so should we hold it back a while and thus give our client the
+ * chance to do its own tdb_lock?
+ */
+
+static bool ctdb_held_back(struct ctdb_db_context *db, TDB_DATA key,
+                          struct ctdb_req_header *hdr)
+{
+       int i;
+       size_t num_keys = talloc_array_length(db->holdback_keys);
+       size_t num_hdrs;
+       struct ctdb_req_header **tmp;
+
+       for (i=0; i<num_keys; i++) {
+               uint8_t *hold_key = db->holdback_keys[i];
+               size_t keylength = talloc_array_length(hold_key);
+
+               if ((keylength == key.dsize)
+                   && (memcmp(hold_key, key.dptr, keylength) == 0)) {
+                       break;
+               }
+       }
+       if (i == num_keys) {
+               return false;
+       }
+       DEBUG(DEBUG_DEBUG, ("holding back record %08x after migration\n",
+                           ctdb_hash(&key)));
+
+       num_hdrs = talloc_array_length(db->held_back);
+
+       tmp = talloc_realloc(db, db->held_back, struct ctdb_req_header *,
+                            num_hdrs + 1);
+       if (tmp == NULL) {
+               DEBUG(DEBUG_ERR, (__location__ "talloc_realloc failed\n"));
+               return false;
+       }
+       db->held_back = tmp;
+       db->held_back[num_hdrs] = talloc_move(db->held_back, &hdr);
+       return true;
+}
 
 /*
   called when a CTDB_REQ_CALL packet comes in
@@ -433,6 +500,13 @@ void ctdb_request_call(struct ctdb_context *ctdb, struct ctdb_req_header *hdr)
                ctdb->statistics.max_hop_count = c->hopcount;
        }
 
+       if ((c->flags & CTDB_IMMEDIATE_MIGRATION)
+           && (ctdb_held_back(ctdb_db, call->key, hdr))) {
+               talloc_free(data.dptr);
+               ctdb_ltdb_unlock(ctdb_db, call->key);
+               return;
+       }
+
        /* if this nodes has done enough consecutive calls on the same record
           then give them the record
           or if the node requested an immediate migration
@@ -792,3 +866,112 @@ void ctdb_send_keepalive(struct ctdb_context *ctdb, uint32_t destnode)
 
        talloc_free(r);
 }
+
+static void ctdb_holdback_cleanup(struct event_context *ev,
+                                 struct timed_event *te,
+                                 struct timeval t, void *private_data)
+{
+       struct ctdb_context *ctdb = talloc_get_type(private_data,
+                                                   struct ctdb_context);
+        struct ctdb_db_context *ctdb_db;
+
+       DEBUG(DEBUG_INFO, ("running ctdb_holdback_cleanup\n"));
+
+       if (te != ctdb->holdback_cleanup_te) {
+               ctdb_fatal(ctdb, "te != ctdb->holdback_cleanup_te");
+       }
+
+        for (ctdb_db=ctdb->db_list; ctdb_db; ctdb_db=ctdb_db->next) {
+               size_t i, num_heldback;
+
+               talloc_free(ctdb_db->holdback_keys);
+               ctdb_db->holdback_keys = NULL;
+
+               num_heldback = talloc_array_length(ctdb_db->held_back);
+               for (i=0; i<num_heldback; i++) {
+                       ctdb_queue_packet(ctdb, ctdb_db->held_back[i]);
+               }
+               talloc_free(ctdb_db->held_back);
+               ctdb_db->held_back = NULL;
+        }
+
+       ctdb->holdback_cleanup_te = event_add_timed(
+               ctdb->ev, ctdb, timeval_current_ofs(
+                       0, ctdb->tunable.holdback_cleanup_interval * 1000),
+               ctdb_holdback_cleanup, ctdb);
+}
+
+void ctdb_start_holdback_cleanup(struct ctdb_context *ctdb)
+{
+       ctdb->holdback_cleanup_te = event_add_timed(
+               ctdb->ev, ctdb, timeval_current_ofs(
+                       0, ctdb->tunable.holdback_cleanup_interval * 1000),
+               ctdb_holdback_cleanup, ctdb);
+
+       CTDB_NO_MEMORY_FATAL(ctdb, ctdb->holdback_cleanup_te);
+
+       DEBUG(DEBUG_NOTICE,("Holdback cleanup has been started\n"));
+}
+
+void ctdb_stop_holdback_cleanup(struct ctdb_context *ctdb)
+{
+       talloc_free(ctdb->holdback_cleanup_te);
+       ctdb->holdback_cleanup_te = NULL;
+}
+
+int32_t ctdb_control_gotit(struct ctdb_context *ctdb, TDB_DATA indata)
+{
+       struct ctdb_control_gotit *c =
+               (struct ctdb_control_gotit *)indata.dptr;
+       struct ctdb_db_context *db;
+       size_t i, num_keys, num_heldback;
+       TDB_DATA key;
+
+       if (indata.dsize < sizeof(struct ctdb_control_gotit)) {
+               DEBUG(DEBUG_ERR, (__location__ "Invalid data size %d\n",
+                                 (int)indata.dsize));
+               return -1;
+       }
+       db = find_ctdb_db(ctdb, c->db_id);
+       if (db == NULL) {
+               DEBUG(DEBUG_ERR, ("Unknown db_id 0x%x in ctdb_reply_dmaster\n",
+                                 (int)c->db_id));
+               return -1;
+       }
+
+       key.dptr = c->key;
+       key.dsize = indata.dsize - offsetof(struct ctdb_control_gotit, key);
+
+       num_keys = talloc_array_length(db->holdback_keys);
+       for (i=0; i<num_keys; i++) {
+               uint8_t *hold_key = db->holdback_keys[i];
+               size_t keylength = talloc_array_length(hold_key);
+
+               if ((keylength == key.dsize)
+                   && (memcmp(hold_key, key.dptr, keylength) == 0)) {
+                       break;
+               }
+       }
+       if (i == num_keys) {
+               /*
+                * ctdb_holdback_cleanup has kicked in. This is okay,
+                * we will just potentially have to retry.
+                */
+               return 0;
+       }
+
+       talloc_free(db->holdback_keys[i]);
+       if (i < num_keys-1) {
+               db->holdback_keys[i] = db->holdback_keys[num_keys-1];
+       }
+       db->holdback_keys = talloc_realloc(db, db->holdback_keys, uint8_t *,
+                                          num_keys-1);
+
+       num_heldback = talloc_array_length(db->held_back);
+       for (i=0; i<num_heldback; i++) {
+               ctdb_queue_packet(ctdb, db->held_back[i]);
+       }
+       talloc_free(db->held_back);
+       db->held_back = NULL;
+       return 0;
+}
index 3382fae39aac964481068c369021b3588dc13437..2b703e73151cb63a637b871b8501d75d6433ee34 100644 (file)
@@ -281,6 +281,7 @@ static int32_t ctdb_control_dispatch(struct ctdb_context *ctdb,
        case CTDB_CONTROL_SHUTDOWN:
                ctdb_stop_recoverd(ctdb);
                ctdb_stop_keepalive(ctdb);
+               ctdb_stop_holdback_cleanup(ctdb);
                ctdb_stop_monitoring(ctdb);
                ctdb_release_all_ips(ctdb);
                if (ctdb->methods != NULL) {
@@ -560,6 +561,9 @@ static int32_t ctdb_control_dispatch(struct ctdb_context *ctdb,
                CHECK_CONTROL_DATA_SIZE(sizeof(uint64_t));
                return ctdb_control_get_db_seqnum(ctdb, indata, outdata);
 
+       case CTDB_CONTROL_GOTIT:
+               return ctdb_control_gotit(ctdb, indata);
+
        default:
                DEBUG(DEBUG_CRIT,(__location__ " Unknown CTDB control opcode %u\n", opcode));
                return -1;
index 9ade55aca86bc66ef1cca45c44402c2081b6dd16..c0e96168b584d9500ef9baba691910995b55d5f5 100644 (file)
@@ -75,6 +75,9 @@ static void ctdb_start_transport(struct ctdb_context *ctdb)
        /* start periodic update of tcp tickle lists */
                ctdb_start_tcp_tickle_update(ctdb);
 
+       /* start periodic cleanup of holdback cleanup */
+       ctdb_start_holdback_cleanup(ctdb);
+
        /* start listening for recovery daemon pings */
        ctdb_control_recd_ping(ctdb);
 }
index 8568e8bbe07e59984f8255ab0dd9b31f6b7aa4c6..fcf3488cb67d71cda41f56af9e3af072ceac23f7 100644 (file)
@@ -1158,6 +1158,7 @@ static void ctdb_recd_ping_timeout(struct event_context *ev, struct timed_event
 
        ctdb_stop_recoverd(ctdb);
        ctdb_stop_keepalive(ctdb);
+       ctdb_stop_holdback_cleanup(ctdb);
        ctdb_stop_monitoring(ctdb);
        ctdb_release_all_ips(ctdb);
        if (ctdb->methods != NULL) {
index 071c0a3da9460191aacd487e44cc6d1463d2377c..752fd4752c29b7437bb7e689a22b3fad36513a45 100644 (file)
@@ -3346,6 +3346,7 @@ static void ctdb_check_recd(struct event_context *ev, struct timed_event *te,
 
                ctdb_stop_recoverd(ctdb);
                ctdb_stop_keepalive(ctdb);
+               ctdb_stop_holdback_cleanup(ctdb);
                ctdb_stop_monitoring(ctdb);
                ctdb_release_all_ips(ctdb);
                if (ctdb->methods != NULL) {
index 6f657e61dae6b949edbcd17889374e024fe64949..91683ce884d71afe4ed65aee5bbfff8af53cf1d3 100644 (file)
@@ -30,6 +30,8 @@ static const struct {
        { "TraverseTimeout",     20, offsetof(struct ctdb_tunable, traverse_timeout) },
        { "KeepaliveInterval",    5,  offsetof(struct ctdb_tunable, keepalive_interval) },
        { "KeepaliveLimit",       5,  offsetof(struct ctdb_tunable, keepalive_limit) },
+       { "HoldbackCleanupInterval", 1000,
+         offsetof(struct ctdb_tunable, holdback_cleanup_interval) },
        { "MaxLACount",           7,  offsetof(struct ctdb_tunable, max_lacount) },
        { "RecoverTimeout",      20,  offsetof(struct ctdb_tunable, recover_timeout) },
        { "RecoverInterval",      1,  offsetof(struct ctdb_tunable, recover_interval) },