Fix persistent transaction commit race condition.

In ctdb_client.c:ctdb_transaction_commit(), after a failed
TRANS2_COMMIT control call (for instance due to the 1-second
being exceeded waiting for a busy node's reply), there is a
1-second gap between the transaction_cancel() and
replay_transaction() calls in which there is no lock on the
persistent db. And due to the lack of global state
indicating that a transaction is in progress in ctdbd, other nodes
may succeed to start transactions on the db in this gap and
even worse work on top of the possibly already pushed changes.
So the data diverges on the several nodes.

This change fixes this by introducing global state for a transaction
commit being active in the ctdb_db_context struct and in a db_id field
in the client so that a client keeps track of _which_ tdb it as
transaction commit running on. These data are set by ctdb upon
entering the trans2_commit control and they are cleared in the
trans2_error or trans2_finished controls. This makes it impossible
to start a nother transaction or migrate a record to a different
node while a transaction is active on a persistent tdb, including
the retry loop.

This approach is dead lock free and still allows recovery process
to be started in the retry-gap between cancel and replay.
Also note, that this solution does not require any change in the
client side.

This was debugged and developed together with
Stefan Metzmacher <metze@samba.org> - thanks!

Michael

(This used to be ctdb commit f88103516e5ad723062fb95fcb07a128f1069d69)

1 files changed, 9 insertions, 0 deletions

diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index 1a94cfc661..fcfba669a0 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -176,6 +176,8 @@ int daemon_deregister_message_handler(struct ctdb_context *ctdb, uint32_t client
 */
 static int ctdb_client_destructor(struct ctdb_client *client)
 {
+	struct ctdb_db_context *ctdb_db;
+
 	ctdb_takeover_client_destructor_hook(client);
 	ctdb_reqid_remove(client->ctdb, client->client_id);
 	if (client->ctdb->statistics.num_clients) {
@@ -186,6 +188,13 @@ static int ctdb_client_destructor(struct ctdb_client *client)
 		DEBUG(DEBUG_ERR,(__location__ " Client disconnecting with %u persistent updates in flight. Starting recovery\n", client->num_persistent_updates));
 		client->ctdb->recovery_mode = CTDB_RECOVERY_ACTIVE;
 	}
+	ctdb_db = find_ctdb_db(client->ctdb, client->db_id);
+	if (ctdb_db) {
+		DEBUG(DEBUG_ERR, (__location__ " client exit while transaction "
+				  "commit active. Forcing recovery.\n"));
+		client->ctdb->recovery_mode = CTDB_RECOVERY_ACTIVE;
+		ctdb_db->transaction_active = false;
+	}
 
 	return 0;
 }