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 08/13] journal: introduce proper error codes
Date: Fri, 11 Jun 2021 23:56:16 +0200
Message-ID: <255747b5470b3cffc28176ead3f6deab4f6b4580.1623448465.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1623448465.git.v.shpilevoy@tarantool.org>

Journal used to have only one error code in journal_entry.res:
-1. It had at least 2 problems:

- There was an assumption that TXN_SIGNATURE_ROLLBACK is the same
  as journal_entry error = -1;

- It wasn't possible to tell if the entry tried to be written and
  failed, or it didn't try yet. Both looked as -1.

The patch introduces a new error code JOURNAL_ENTRY_ERR_UNKNOWN.
The IO error now has its own value: JOURNAL_ENTRY_ERR_IO.

This helps to ensure that a not finished journal entry or a
transaction won't try to obtain a diag error for their result.

Part of #6027
---
 src/box/journal.c   |  1 +
 src/box/journal.h   | 15 ++++++++++++++-
 src/box/txn.c       |  6 ++++--
 src/box/txn.h       | 13 ++++++++++---
 src/box/txn_limbo.c |  5 +++--
 src/box/wal.c       |  4 ++--
 6 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/src/box/journal.c b/src/box/journal.c
index 0a1e9932a..35c99c1c6 100644
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -45,6 +45,7 @@ struct journal_queue journal_queue = {
 void
 diag_set_journal_res_detailed(const char *file, unsigned line, int64_t res)
 {
+	assert(res < 0 && res != JOURNAL_ENTRY_ERR_UNKNOWN);
 	(void)res;
 	diag_set_detailed(file, line, ClientError, ER_WAL_IO);
 }
diff --git a/src/box/journal.h b/src/box/journal.h
index 01ea60f72..18767176e 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -44,6 +44,19 @@ struct journal_entry;
 
 typedef void (*journal_write_async_f)(struct journal_entry *entry);
 
+enum {
+	/** Entry didn't attempt a journal write. */
+	JOURNAL_ENTRY_ERR_UNKNOWN = -1,
+	/** Tried to be written, but something happened related to IO. */
+	JOURNAL_ENTRY_ERR_IO = -2,
+	/**
+	 * Anchor for the structs built on top of journal entry so as they
+	 * could introduce their own unique errors. Set to a big value in
+	 * advance.
+	 */
+	JOURNAL_ENTRY_ERR_MIN = -100,
+};
+
 /**
  * Convert a result of a journal entry write to an error installed into the
  * current diag.
@@ -108,7 +121,7 @@ journal_entry_create(struct journal_entry *entry, size_t n_rows,
 	entry->complete_data	= complete_data;
 	entry->approx_len	= approx_len;
 	entry->n_rows		= n_rows;
-	entry->res		= -1;
+	entry->res		= JOURNAL_ENTRY_ERR_UNKNOWN;
 	entry->flags		= 0;
 }
 
diff --git a/src/box/txn.c b/src/box/txn.c
index 2c889a31a..c9c2e93ff 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -276,7 +276,7 @@ txn_begin(void)
 	txn->psn = 0;
 	txn->rv_psn = 0;
 	txn->status = TXN_INPROGRESS;
-	txn->signature = TXN_SIGNATURE_ROLLBACK;
+	txn->signature = TXN_SIGNATURE_UNKNOWN;
 	txn->engine = NULL;
 	txn->engine_tx = NULL;
 	txn->fk_deferred_count = 0;
@@ -479,6 +479,7 @@ txn_complete_fail(struct txn *txn)
 {
 	assert(!txn_has_flag(txn, TXN_IS_DONE));
 	assert(txn->signature < 0);
+	assert(txn->signature != TXN_SIGNATURE_UNKNOWN);
 	txn->status = TXN_ABORTED;
 	struct txn_stmt *stmt;
 	stailq_reverse(&txn->stmts);
@@ -535,7 +536,8 @@ txn_on_journal_write(struct journal_entry *entry)
 	 * txn_limbo has already rolled the tx back, so we just
 	 * have to free it.
 	 */
-	if (txn->signature < TXN_SIGNATURE_ROLLBACK) {
+	if (txn->signature != TXN_SIGNATURE_UNKNOWN) {
+		assert(txn->signature < 0);
 		txn_free(txn);
 		return;
 	}
diff --git a/src/box/txn.h b/src/box/txn.h
index d51761bc9..037865ac6 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -36,6 +36,7 @@
 #include "trigger.h"
 #include "fiber.h"
 #include "space.h"
+#include "journal.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -103,24 +104,30 @@ enum {
 enum {
 	/** Signature set for empty transactions. */
 	TXN_SIGNATURE_NOP = 0,
+	/**
+	 * Aliases for journal errors to make all signature codes have the same
+	 * prefix.
+	 */
+	TXN_SIGNATURE_UNKNOWN = JOURNAL_ENTRY_ERR_UNKNOWN,
+	TXN_SIGNATURE_IO = JOURNAL_ENTRY_ERR_IO,
 	/**
 	 * The default signature value for failed transactions.
 	 * Indicates either write failure or any other failure
 	 * not caused by synchronous transaction processing.
 	 */
-	TXN_SIGNATURE_ROLLBACK = -1,
+	TXN_SIGNATURE_ROLLBACK = JOURNAL_ENTRY_ERR_MIN - 1,
 	/**
 	 * A value set for failed synchronous transactions
 	 * on master, when not enough acks were collected.
 	 */
-	TXN_SIGNATURE_QUORUM_TIMEOUT = -2,
+	TXN_SIGNATURE_QUORUM_TIMEOUT = JOURNAL_ENTRY_ERR_MIN - 2,
 	/**
 	 * A value set for failed synchronous transactions
 	 * on replica (or any instance during recovery), when a
 	 * transaction is rolled back because ROLLBACK message was
 	 * read.
 	 */
-	TXN_SIGNATURE_SYNC_ROLLBACK = -3,
+	TXN_SIGNATURE_SYNC_ROLLBACK = JOURNAL_ENTRY_ERR_MIN - 3,
 };
 
 /**
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index b03c71514..51dc2a186 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -79,7 +79,8 @@ txn_limbo_append(struct txn_limbo *limbo, uint32_t id, struct txn *txn)
 	 * needs that to be able rollback transactions, whose WAL write is in
 	 * progress.
 	 */
-	assert(txn->signature < 0);
+	assert(txn->signature == TXN_SIGNATURE_UNKNOWN);
+	assert(txn->status == TXN_PREPARED);
 	if (limbo->is_in_rollback) {
 		/*
 		 * Cascading rollback. It is impossible to commit the
@@ -393,7 +394,7 @@ txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn)
 			 */
 			if (e->lsn == -1)
 				break;
-		} else if (e->txn->signature < 0) {
+		} else if (e->txn->signature == TXN_SIGNATURE_UNKNOWN) {
 			/*
 			 * A transaction might be covered by the CONFIRM even if
 			 * it is not written to WAL yet when it is an async
diff --git a/src/box/wal.c b/src/box/wal.c
index 40382e791..f59cc9113 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -1169,7 +1169,7 @@ done:
 	if (!stailq_empty(&rollback)) {
 		/* Update status of the successfully committed requests. */
 		stailq_foreach_entry(entry, &rollback, fifo)
-			entry->res = -1;
+			entry->res = JOURNAL_ENTRY_ERR_IO;
 		/* Rollback unprocessed requests */
 		stailq_concat(&wal_msg->rollback, &rollback);
 		wal_begin_rollback();
@@ -1293,7 +1293,7 @@ wal_write_async(struct journal *journal, struct journal_entry *entry)
 	return 0;
 
 fail:
-	entry->res = -1;
+	assert(entry->res == JOURNAL_ENTRY_ERR_UNKNOWN);
 	return -1;
 }
 
-- 
2.24.3 (Apple Git-128)


  parent reply	other threads:[~2021-06-11 22:03 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 ` [Tarantool-patches] [PATCH 03/13] journal: make journal_write() set diag on error Vladislav Shpilevoy via Tarantool-patches
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 ` Vladislav Shpilevoy via Tarantool-patches [this message]
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=255747b5470b3cffc28176ead3f6deab4f6b4580.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