[Tarantool-patches] [PATCH 3/3] box/txn: clear fiber storage right before journal_write

[Cyrill Gorcunov]

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 <gorcunov at gmail.com>
---
 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 <small/region.h>
 #include <diag.h>
 
@@ -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