[Tarantool-patches] [PATCH 08/13] journal: introduce proper error codes

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.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)