Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com,
	sergepetrenko@tarantool.org
Subject: [Tarantool-patches] [PATCH 03/13] journal: make journal_write() set diag on error
Date: Fri, 11 Jun 2021 23:56:11 +0200
Message-ID: <3cf8acc10c635d7a7b84ea1869213a2d11cf4ec9.1623448465.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1623448465.git.v.shpilevoy@tarantool.org>

It used to simply return -1 and set a diag only when OOM happened
inside.

The caller was forced either to ignore the result or set its own
diag regardless of what really happened.

The patch makes journal_write() set a correct diag error when it
returns -1. The only implementation to change was
wal_write_async(). The other implementations always return 0.

Part of #6027
---
 src/box/applier.cc  |  4 +++-
 src/box/raft.c      |  8 +++++---
 src/box/txn.c       | 14 ++++++++------
 src/box/txn_limbo.c | 30 +++++++++++++++++-------------
 src/box/wal.c       |  2 ++
 5 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/src/box/applier.cc b/src/box/applier.cc
index 33181fdbf..60d648795 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -835,7 +835,9 @@ apply_synchro_row(struct xrow_header *row)
 	 * before trying to commit. But that requires extra steps from the
 	 * transactions side, including the async ones.
 	 */
-	if (journal_write(&entry.base) != 0 || entry.base.res < 0) {
+	if (journal_write(&entry.base) != 0)
+		goto err;
+	if (entry.base.res < 0) {
 		diag_set(ClientError, ER_WAL_IO);
 		goto err;
 	}
diff --git a/src/box/raft.c b/src/box/raft.c
index 6b52c9876..55dee4cb1 100644
--- a/src/box/raft.c
+++ b/src/box/raft.c
@@ -307,17 +307,19 @@ box_raft_write(struct raft *raft, const struct raft_msg *msg)
 	 * follows this pattern of 'protection'.
 	 */
 	bool cancellable = fiber_set_cancellable(false);
-	bool ok = (journal_write(entry) == 0 && entry->res >= 0);
+	bool is_err = journal_write(entry) != 0;
 	fiber_set_cancellable(cancellable);
-	if (!ok) {
+	if (is_err)
+		goto fail;
+	if (entry->res < 0) {
 		diag_set(ClientError, ER_WAL_IO);
-		diag_log();
 		goto fail;
 	}
 
 	region_truncate(region, svp);
 	return;
 fail:
+	diag_log();
 	/*
 	 * XXX: the stub is supposed to be removed once it is defined what to do
 	 * when a raft request WAL write fails.
diff --git a/src/box/txn.c b/src/box/txn.c
index 966dfafdf..761630939 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -847,7 +847,6 @@ txn_commit_try_async(struct txn *txn)
 	fiber_set_txn(fiber(), NULL);
 	if (journal_write_try_async(req) != 0) {
 		fiber_set_txn(fiber(), txn);
-		diag_set(ClientError, ER_WAL_IO);
 		diag_log();
 		goto rollback;
 	}
@@ -904,12 +903,11 @@ txn_commit(struct txn *txn)
 	}
 
 	fiber_set_txn(fiber(), NULL);
-	if (journal_write(req) != 0 || req->res < 0) {
-		if (txn_has_flag(txn, TXN_WAIT_SYNC))
-			txn_limbo_abort(&txn_limbo, limbo_entry);
+	if (journal_write(req) != 0)
+		goto rollback_io;
+	if (req->res < 0) {
 		diag_set(ClientError, ER_WAL_IO);
-		diag_log();
-		goto rollback;
+		goto rollback_io;
 	}
 	if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
 		if (txn_has_flag(txn, TXN_WAIT_ACK)) {
@@ -934,6 +932,10 @@ txn_commit(struct txn *txn)
 	txn_free(txn);
 	return 0;
 
+rollback_io:
+	diag_log();
+	if (txn_has_flag(txn, TXN_WAIT_SYNC))
+		txn_limbo_abort(&txn_limbo, limbo_entry);
 rollback:
 	assert(txn->fiber != NULL);
 	if (!txn_has_flag(txn, TXN_IS_DONE)) {
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index dae6d2df4..83b86387c 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -335,21 +335,25 @@ txn_limbo_write_synchro(struct txn_limbo *limbo, uint16_t type, int64_t lsn,
 	journal_entry_create(entry, 1, xrow_approx_len(&row),
 			     journal_entry_fiber_wakeup_cb, fiber());
 
-	if (journal_write(entry) != 0 || entry->res < 0) {
+	if (journal_write(entry) != 0)
+		goto fail;
+	if (entry->res < 0) {
 		diag_set(ClientError, ER_WAL_IO);
-		diag_log();
-		/*
-		 * XXX: the stub is supposed to be removed once it is defined
-		 * what to do when a synchro request WAL write fails. One of
-		 * the possible solutions: log the error, keep the limbo
-		 * queue as is and probably put in rollback mode. Then
-		 * provide a hook to call manually when WAL problems are fixed.
-		 * Or retry automatically with some period.
-		 */
-		panic("Could not write a synchro request to WAL: "
-		      "lsn = %lld, type = %s\n", (long long)lsn,
-		      iproto_type_name(type));
+		goto fail;
 	}
+	return;
+fail:
+	diag_log();
+	/*
+	 * XXX: the stub is supposed to be removed once it is defined what to do
+	 * when a synchro request WAL write fails. One of the possible
+	 * solutions: log the error, keep the limbo queue as is and probably put
+	 * in rollback mode. Then provide a hook to call manually when WAL
+	 * problems are fixed. Or retry automatically with some period.
+	 */
+	panic("Could not write a synchro request to WAL: lsn = %lld, "
+	      "type = %s\n", (long long)lsn, iproto_type_name(type));
+
 }
 
 /**
diff --git a/src/box/wal.c b/src/box/wal.c
index 5c52142ef..25edbace6 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -1244,6 +1244,7 @@ wal_write_async(struct journal *journal, struct journal_entry *entry)
 	struct wal_writer *writer = (struct wal_writer *) journal;
 
 	ERROR_INJECT(ERRINJ_WAL_IO, {
+		diag_set(ClientError, ER_WAL_IO);
 		goto fail;
 	});
 
@@ -1258,6 +1259,7 @@ wal_write_async(struct journal *journal, struct journal_entry *entry)
 		say_error("Aborting transaction %lld during "
 			  "cascading rollback",
 			  (long long)vclock_sum(&writer->vclock));
+		diag_set(ClientError, ER_CASCADE_ROLLBACK);
 		goto fail;
 	}
 
-- 
2.24.3 (Apple Git-128)


  parent reply	other threads:[~2021-06-11 22:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 21:56 [Tarantool-patches] [PATCH 00/13] Applier rollback reason Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 01/13] error: introduce ER_CASCADE_ROLLBACK Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 10/13] txn: install proper diag errors on txn fail Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 11/13] wal: introduce JOURNAL_ENTRY_ERR_CASCADE Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 12/13] txn: introduce TXN_SIGNATURE_ABORT Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 13/13] txn: stop TXN_SIGNATURE_ABORT override Vladislav Shpilevoy via Tarantool-patches
2021-06-15 13:44   ` Serge Petrenko via Tarantool-patches
2021-06-15 19:34     ` Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 02/13] test: remove replica-applier-rollback.lua Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 04/13] wal: refactor wal_write_to_disk() Vladislav Shpilevoy via Tarantool-patches
2021-06-15 20:46   ` Cyrill Gorcunov via Tarantool-patches
2021-06-16  6:22     ` Vladislav Shpilevoy via Tarantool-patches
2021-06-16  8:02       ` Cyrill Gorcunov via Tarantool-patches
2021-06-16 23:32         ` Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 05/13] diag: introduce diag_set_detailed() Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 06/13] wal: encapsulate ER_WAL_IO Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 07/13] txn: change limbo rollback check in the trigger Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 08/13] journal: introduce proper error codes Vladislav Shpilevoy via Tarantool-patches
2021-06-11 21:56 ` [Tarantool-patches] [PATCH 09/13] txn: assert after WAL write that txn is not done Vladislav Shpilevoy via Tarantool-patches
2021-06-15 13:43 ` [Tarantool-patches] [PATCH 00/13] Applier rollback reason Serge Petrenko via Tarantool-patches
2021-06-16 23:32 ` Vladislav Shpilevoy via Tarantool-patches

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=3cf8acc10c635d7a7b84ea1869213a2d11cf4ec9.1623448465.git.v.shpilevoy@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /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

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git