From 4d896f84d801e47af70439a713be1131871b1675 Mon Sep 17 00:00:00 2001
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date: Wed, 9 Jun 2021 23:19:14 +0200
Subject: [PATCH] journal: introduce proper error codes
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 0a1e9932a3..35c99c1c6b 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 01ea60f722..18767176e8 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 2c889a31a4..c9c2e93ff4 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 d51761bc9c..037865ac64 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 b03c715147..51dc2a186c 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 cb8b2eac1c..6f3651fc1c 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -1170,7 +1170,7 @@ wal_write_to_disk(struct cmsg *msg)
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();
@@ -1294,7 +1294,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;
}
--
GitLab