Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, sergepetrenko@tarantool.org,
	gorcunov@gmail.com
Subject: [Tarantool-patches] [PATCH 1/3] txn: rename txn_complete_async to txn_on_journal_write
Date: Sat, 31 Oct 2020 19:01:40 +0100	[thread overview]
Message-ID: <b994d8b4977b1d78c64a3595d665c821b92ca626.1604166646.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1604166646.git.v.shpilevoy@tarantool.org>

The function is called only by the journal when write is finished.

Besides, it may not complete the transaction. In case of
synchronous replication it is not enough for completion. It means,
it can't have 'complete' in its name.

Also the function is never used out of txn.c, so it is removed
from txn.h and is now static.

The patch is a preparation for not spaming "too long WAL write" on
synchronous transactions, because it is simply misleading.

Part of #5139
---
 src/box/txn.c       |  7 ++++---
 src/box/txn.h       |  6 ------
 src/box/txn_limbo.c | 29 ++++++++++++++++++++++-------
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/src/box/txn.c b/src/box/txn.c
index eb725aaa9..8aedcc9e4 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -580,8 +580,9 @@ txn_complete(struct txn *txn)
 	}
 }
 
-void
-txn_complete_async(struct journal_entry *entry)
+/** Callback invoked when the transaction's journal write is finished. */
+static void
+txn_on_journal_write(struct journal_entry *entry)
 {
 	struct txn *txn = entry->complete_data;
 	/*
@@ -614,7 +615,7 @@ txn_journal_entry_new(struct txn *txn)
 
 	/* Save space for an additional NOP row just in case. */
 	req = journal_entry_new(txn->n_new_rows + txn->n_applier_rows + 1,
-				&txn->region, txn_complete_async, txn);
+				&txn->region, txn_on_journal_write, txn);
 	if (req == NULL)
 		return NULL;
 
diff --git a/src/box/txn.h b/src/box/txn.h
index a51bdf8e6..ffe240867 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -441,12 +441,6 @@ txn_commit(struct txn *txn);
 void
 txn_rollback(struct txn *txn);
 
-/**
- * Complete asynchronous transaction.
- */
-void
-txn_complete_async(struct journal_entry *entry);
-
 /**
  * Submit a transaction to the journal.
  * @pre txn == in_txn()
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index 908a17fcc..06a3d30c3 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -373,10 +373,17 @@ txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn)
 		txn_clear_flag(e->txn, TXN_WAIT_SYNC);
 		txn_clear_flag(e->txn, TXN_WAIT_ACK);
 		/*
-		 * If  txn_complete_async() was already called,
-		 * finish tx processing. Otherwise just clear the
-		 * "WAIT_ACK" flag. Tx procesing will finish once
-		 * the tx is written to WAL.
+		 * If already written to WAL by now, finish tx processing.
+		 * Otherwise just clear the sync flags. Tx procesing will finish
+		 * automatically once the tx is written to WAL.
+		 *
+		 * XXX: Normally at this point all transactions covered by this
+		 * CONFIRM should be in WAL already, but there is a bug, that
+		 * replica always processes received synchro requests *before*
+		 * writing them to WAL. So it can happen, that a CONFIRM is
+		 * 'read', but the transaction is not written yet. Should be
+		 * fixed when the replica will behave properly, and then this
+		 * branch won't exist.
 		 */
 		if (e->txn->signature >= 0)
 			txn_complete(e->txn);
@@ -424,9 +431,17 @@ txn_limbo_read_rollback(struct txn_limbo *limbo, int64_t lsn)
 			txn_complete(e->txn);
 		} else {
 			/*
-			 * Rollback the transaction, but don't
-			 * free it yet. txn_complete_async() will
-			 * free it.
+			 * Rollback the transaction, but don't free it yet. It
+			 * will be freed after its WAL write is completed.
+			 *
+			 * XXX: Normally at this point all transactions covered
+			 * by this ROLLBACK should be in WAL already, but there
+			 * is a bug, that replica always processes received
+			 * synchro requests *before* writing them to WAL. So it
+			 * can happen, that a ROLLBACK is 'read', but the
+			 * transaction is not written yet. Should be fixed when
+			 * the replica will behave properly, and then this
+			 * branch won't exist.
 			 */
 			e->txn->signature = TXN_SIGNATURE_SYNC_ROLLBACK;
 			struct fiber *fiber = e->txn->fiber;
-- 
2.21.1 (Apple Git-122.3)

  reply	other threads:[~2020-10-31 18:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-31 18:01 [Tarantool-patches] [PATCH 0/3] Qsync too long WAL write Vladislav Shpilevoy
2020-10-31 18:01 ` Vladislav Shpilevoy [this message]
2020-11-02 11:48   ` [Tarantool-patches] [PATCH 1/3] txn: rename txn_complete_async to txn_on_journal_write Cyrill Gorcunov
2020-10-31 18:01 ` [Tarantool-patches] [PATCH 2/3] txn: split complete into success and fail paths Vladislav Shpilevoy
2020-11-02 12:15   ` Cyrill Gorcunov
2020-11-02 22:39     ` Vladislav Shpilevoy
2020-10-31 18:01 ` [Tarantool-patches] [PATCH 3/3] txn: warn "too long WAL" on write, not on commit Vladislav Shpilevoy
2020-11-02 12:31   ` Cyrill Gorcunov
2020-11-03  7:36 ` [Tarantool-patches] [PATCH 0/3] Qsync too long WAL write Serge Petrenko
2020-11-03 22:19 ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b994d8b4977b1d78c64a3595d665c821b92ca626.1604166646.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 1/3] txn: rename txn_complete_async to txn_on_journal_write' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox