[Tarantool-patches] [PATCH 08/11] qsync: txn_limbo_wait_confirm -- refactor code a bit

Cyrill Gorcunov gorcunov at gmail.com
Thu Nov 12 22:51:18 MSK 2020


 - no need for useless comments which describe what
   we're doing, it is obvious from the code, instead
   put comment explaining "why" we're doing these
   things;
 - use designated assignments for cwp, this is a rule
   of thumb for C where structure need to be initialized
   from the scratch;
 - reorder triggers declaration mess, without empty lines
   which are ordering context they are simply unreadable;
 - try to not use goto/jumps when possible (gotos are
   suitable as a common error entry point but here we
   can handle everything inside cycle itself).

Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
 src/box/txn_limbo.c | 54 ++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index b58b9647d..cf6122360 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -573,20 +573,25 @@ txn_limbo_wait_confirm(struct txn_limbo *limbo)
 	if (txn_limbo_is_empty(limbo))
 		return 0;
 
-	/* initialization of a waitpoint. */
-	struct confirm_waitpoint cwp;
-	cwp.caller = fiber();
-	cwp.is_confirm = false;
-	cwp.is_rollback = false;
-
-	/* Set triggers for the last limbo transaction. */
-	struct trigger on_complete;
+	struct confirm_waitpoint cwp = {
+		.caller = fiber(),
+		.is_confirm = false,
+		.is_rollback = false,
+	};
+
+	/*
+	 * Since we're waiting for all sync transactions to complete,
+	 * we need the last entry from the limbo.
+	 */
+	struct txn_limbo_entry *tle = txn_limbo_last_entry(limbo);
+
+	struct trigger on_complete, on_rollback;
 	trigger_create(&on_complete, txn_commit_cb, &cwp, NULL);
-	struct trigger on_rollback;
 	trigger_create(&on_rollback, txn_rollback_cb, &cwp, NULL);
-	struct txn_limbo_entry *tle = txn_limbo_last_entry(limbo);
+
 	txn_on_commit(tle->txn, &on_complete);
 	txn_on_rollback(tle->txn, &on_rollback);
+
 	double start_time = fiber_clock();
 	while (true) {
 		double deadline = start_time + replication_synchro_timeout;
@@ -594,25 +599,18 @@ txn_limbo_wait_confirm(struct txn_limbo *limbo)
 		double timeout = deadline - fiber_clock();
 		int rc = fiber_cond_wait_timeout(&limbo->wait_cond, timeout);
 		fiber_set_cancellable(cancellable);
-		if (cwp.is_confirm || cwp.is_rollback)
-			goto complete;
-		if (rc != 0)
-			goto timed_out;
-	}
-timed_out:
-	/* Clear the triggers if the timeout has been reached. */
-	trigger_clear(&on_complete);
-	trigger_clear(&on_rollback);
-	diag_set(ClientError, ER_SYNC_QUORUM_TIMEOUT);
-	return -1;
-
-complete:
-	if (!cwp.is_confirm) {
-		/* The transaction has been rolled back. */
-		diag_set(ClientError, ER_SYNC_ROLLBACK);
-		return -1;
+		if (cwp.is_confirm) {
+			return 0;
+		} else if (cwp.is_rollback) {
+			diag_set(ClientError, ER_SYNC_ROLLBACK);
+			return -1;
+		} else if (rc != 0) {
+			trigger_clear(&on_complete);
+			trigger_clear(&on_rollback);
+			diag_set(ClientError, ER_SYNC_QUORUM_TIMEOUT);
+			return -1;
+		}
 	}
-	return 0;
 }
 
 int
-- 
2.26.2



More information about the Tarantool-patches mailing list