From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f67.google.com (mail-lf1-f67.google.com [209.85.167.67]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5762A4696C3 for ; Thu, 27 Feb 2020 17:17:17 +0300 (MSK) Received: by mail-lf1-f67.google.com with SMTP id w27so671565lfc.1 for ; Thu, 27 Feb 2020 06:17:17 -0800 (PST) From: Cyrill Gorcunov Date: Thu, 27 Feb 2020 17:16:28 +0300 Message-Id: <20200227141628.13782-4-gorcunov@gmail.com> In-Reply-To: <20200227141628.13782-1-gorcunov@gmail.com> References: <20200227141628.13782-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 3/3] box/txn: clear fiber storage right before journal_write List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tml Otherwise any previous attempts to rollback the transaction will triggers assert since rollback action requires the fiber to carry txn in the fiber->storage.txn. To make sure the rollback works as expected we inject journal allocation error to happen. There are some nonobvious moments to be clarified. Both asyncronous and synchronous commits are using async commit engine. And journal_entry_complete is critical for both: it handles success writes and rollback on failures. On sync commits (txn_commit) we simply call fiber_yield and wait until we're woken up by journal_entry_complete to proceed. In turn txn_commit_async doesn't wait for commit to happen but leaves journal entry to commit later (for this reason the fiber->storage.txn is set to null). Fixes #4776 Signed-off-by: Cyrill Gorcunov --- src/box/journal.c | 6 ++++++ src/box/txn.c | 2 +- src/lib/core/errinj.h | 3 ++- test/box/errinj.result | 20 ++++++++++++++++++++ test/box/errinj.test.lua | 7 +++++++ 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/box/journal.c b/src/box/journal.c index 11e78990d..9633e2af3 100644 --- a/src/box/journal.c +++ b/src/box/journal.c @@ -29,6 +29,7 @@ * SUCH DAMAGE. */ #include "journal.h" +#include "errinj.h" #include #include @@ -62,6 +63,11 @@ journal_entry_new(size_t n_rows, struct region *region, size_t size = (sizeof(struct journal_entry) + sizeof(entry->rows[0]) * n_rows); + ERROR_INJECT(ERRINJ_JOURNAL_NEW, { + diag_set(OutOfMemory, 0, "region", "journal_entry"); + return NULL; + }); + entry = region_aligned_alloc(region, size, alignof(struct journal_entry)); if (entry == NULL) { diff --git a/src/box/txn.c b/src/box/txn.c index 9e2888c09..8b10c0965 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -520,6 +520,7 @@ txn_write_to_wal(struct txn *txn) assert(local_row == remote_row + txn->n_new_rows); /* Send the entry to the journal. */ + fiber_set_txn(fiber(), NULL); if (journal_write(req) < 0) { diag_set(ClientError, ER_WAL_IO); diag_log(); @@ -583,7 +584,6 @@ txn_commit_async(struct txn *txn) fiber_set_txn(fiber(), NULL); return 0; } - fiber_set_txn(fiber(), NULL); return txn_write_to_wal(txn); } diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h index ed0cba903..08ea47f2e 100644 --- a/src/lib/core/errinj.h +++ b/src/lib/core/errinj.h @@ -136,7 +136,8 @@ struct errinj { _(ERRINJ_SWIM_FD_ONLY, ERRINJ_BOOL, {.bparam = false}) \ _(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT, {.iparam = 0}) \ _(ERRINJ_FIBER_MADVISE, ERRINJ_BOOL, {.bparam = false}) \ - _(ERRINJ_FIBER_MPROTECT, ERRINJ_INT, {.iparam = -1}) + _(ERRINJ_FIBER_MPROTECT, ERRINJ_INT, {.iparam = -1}) \ + _(ERRINJ_JOURNAL_NEW, ERRINJ_BOOL, {.bparam = false}) ENUM0(errinj_id, ERRINJ_LIST); extern struct errinj errinjs[]; diff --git a/test/box/errinj.result b/test/box/errinj.result index daa27ed24..927f86402 100644 --- a/test/box/errinj.result +++ b/test/box/errinj.result @@ -54,6 +54,7 @@ evals - ERRINJ_HTTP_RESPONSE_ADD_WAIT: false - ERRINJ_INDEX_ALLOC: false - ERRINJ_IPROTO_TX_DELAY: false + - ERRINJ_JOURNAL_NEW: false - ERRINJ_LOG_ROTATE: false - ERRINJ_MEMTX_DELAY_GC: false - ERRINJ_PORT_DUMP: false @@ -134,6 +135,25 @@ errinj.set("ERRINJ_TESTING", false) --- - ok ... +-- Check journal entries allocation failure +errinj.set("ERRINJ_JOURNAL_NEW", true) +--- +- ok +... +space:insert{1} +--- +- error: Failed to allocate 0 bytes in region for journal_entry +... +space:get{1} +--- +... +errinj.set("ERRINJ_JOURNAL_NEW", false) +--- +- ok +... +space:truncate() +--- +... -- Check how well we handle a failed log write errinj.set("ERRINJ_WAL_IO", true) --- diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua index 01d2b68df..f007e97f6 100644 --- a/test/box/errinj.test.lua +++ b/test/box/errinj.test.lua @@ -30,6 +30,13 @@ errinj.set("ERRINJ_TESTING", true) space:select{222444} errinj.set("ERRINJ_TESTING", false) +-- Check journal entries allocation failure +errinj.set("ERRINJ_JOURNAL_NEW", true) +space:insert{1} +space:get{1} +errinj.set("ERRINJ_JOURNAL_NEW", false) +space:truncate() + -- Check how well we handle a failed log write errinj.set("ERRINJ_WAL_IO", true) space:insert{1} -- 2.20.1