From 25382617b95722da7a57ed58bbef3ce528177ab8 Mon Sep 17 00:00:00 2001
From: Serge Petrenko <sergepetrenko@tarantool.org>
Date: Thu, 2 Jul 2020 14:20:37 +0300
Subject: [PATCH] replication: append NOP as the last tx row
Since we stopped sending local space operations in replication, the last
tx row has to be global in order to preserve tx boundaries on replica.
If the last row happens to be a local one, replica will never receive
the tx end marker, yielding the following errors:
`ER_UNSUPPORTED: replication does not support interleaving
transactions`.
In order to fix the problem append a global NOP row at the tx end if
it happens to end on a local row.
Follow-up #4114
Closes #4928
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/txn.c | 27 +++-
test/replication/gh-4928-tx-boundaries.result | 138 ++++++++++++++++++
.../gh-4928-tx-boundaries.test.lua | 61 ++++++++
test/replication/suite.cfg | 1 +
4 files changed, 226 insertions(+), 1 deletion(-)
create mode 100644 test/replication/gh-4928-tx-boundaries.result
create mode 100644 test/replication/gh-4928-tx-boundaries.test.lua
diff --git a/src/box/txn.c b/src/box/txn.c
index 123520166a..765dbd2ce5 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -35,6 +35,7 @@
#include <fiber.h>
#include "xrow.h"
#include "errinj.h"
+#include "iproto_constants.h"
double too_long_threshold;
@@ -488,7 +489,8 @@ txn_journal_entry_new(struct txn *txn)
assert(txn->n_new_rows + txn->n_applier_rows > 0);
- req = journal_entry_new(txn->n_new_rows + txn->n_applier_rows,
+ /* Save space for an additional NOP row just in case. */
+ req = journal_entry_new(txn->n_new_rows + txn->n_applier_rows + 1,
&txn->region, txn);
if (req == NULL)
return NULL;
@@ -517,6 +519,29 @@ txn_journal_entry_new(struct txn *txn)
assert(remote_row == req->rows + txn->n_applier_rows);
assert(local_row == remote_row + txn->n_new_rows);
+ /*
+ * Append a dummy NOP statement to preserve replication tx
+ * boundaries when the last tx row is a local one, and the
+ * transaction has at least one global row.
+ */
+ if (txn->n_local_rows > 0 &&
+ (txn->n_local_rows != txn->n_new_rows || txn->n_applier_rows > 0) &&
+ (*(local_row - 1))->group_id == GROUP_LOCAL) {
+ size_t size;
+ *local_row = region_alloc_object(&txn->region,
+ typeof(**local_row), &size);
+ if (*local_row == NULL) {
+ diag_set(OutOfMemory, size, "region_alloc_object",
+ "row");
+ return NULL;
+ }
+ memset(*local_row, 0, sizeof(**local_row));
+ (*local_row)->type = IPROTO_NOP;
+ (*local_row)->group_id = GROUP_DEFAULT;
+ } else {
+ --req->n_rows;
+ }
+
return req;
}
diff --git a/test/replication/gh-4928-tx-boundaries.result b/test/replication/gh-4928-tx-boundaries.result
new file mode 100644
index 0000000000..969bd84386
--- /dev/null
+++ b/test/replication/gh-4928-tx-boundaries.result
@@ -0,0 +1,138 @@
+-- test-run result file version 2
+-- gh-4928. Test that transactions mixing local and global
+-- space operations are replicated correctly.
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+bit = require('bit')
+ | ---
+ | ...
+
+-- Init.
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+_ = box.schema.space.create('glob')
+ | ---
+ | ...
+_ = box.schema.space.create('loc', {is_local=true})
+ | ---
+ | ...
+_ = box.space.glob:create_index('pk')
+ | ---
+ | ...
+_ = box.space.loc:create_index('pk')
+ | ---
+ | ...
+
+function gen_mixed_tx(i)\
+ box.begin()\
+ if bit.band(i, 1) ~= 0 then\
+ box.space.glob:insert{10 * i + 1}\
+ else\
+ box.space.loc:insert{10 * i + 1}\
+ end\
+ if bit.band(i, 2) ~= 0 then\
+ box.space.glob:insert{10 * i + 2}\
+ else\
+ box.space.loc:insert{10 * i + 2}\
+ end\
+ if bit.band(i, 4) ~= 0 then\
+ box.space.glob:insert{10 * i + 3}\
+ else\
+ box.space.loc:insert{10 * i + 3}\
+ end\
+ box.commit()\
+end
+ | ---
+ | ...
+
+test_run:cmd("create server replica with rpl_master=default,\
+ script='replication/replica.lua'")
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica')
+ | ---
+ | - true
+ | ...
+test_run:wait_downstream(2, {status='follow'})
+ | ---
+ | - true
+ | ...
+
+for i = 0, 7 do gen_mixed_tx(i) end
+ | ---
+ | ...
+
+box.info.replication[2].status
+ | ---
+ | - null
+ | ...
+
+vclock = box.info.vclock
+ | ---
+ | ...
+vclock[0] = nil
+ | ---
+ | ...
+test_run:wait_vclock("replica", vclock)
+ | ---
+ | ...
+
+test_run:cmd('switch replica')
+ | ---
+ | - true
+ | ...
+
+box.info.status
+ | ---
+ | - running
+ | ...
+box.info.replication[1].upstream.status
+ | ---
+ | - follow
+ | ...
+
+box.space.glob:select{}
+ | ---
+ | - - [11]
+ | - [22]
+ | - [31]
+ | - [32]
+ | - [43]
+ | - [51]
+ | - [53]
+ | - [62]
+ | - [63]
+ | - [71]
+ | - [72]
+ | - [73]
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+
+-- Cleanup.
+test_run:cmd('stop server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica')
+ | ---
+ | - true
+ | ...
+box.schema.user.revoke('guest', 'replication')
+ | ---
+ | ...
+box.space.loc:drop()
+ | ---
+ | ...
+box.space.glob:drop()
+ | ---
+ | ...
diff --git a/test/replication/gh-4928-tx-boundaries.test.lua b/test/replication/gh-4928-tx-boundaries.test.lua
new file mode 100644
index 0000000000..92526fc51f
--- /dev/null
+++ b/test/replication/gh-4928-tx-boundaries.test.lua
@@ -0,0 +1,61 @@
+-- gh-4928. Test that transactions mixing local and global
+-- space operations are replicated correctly.
+env = require('test_run')
+test_run = env.new()
+bit = require('bit')
+
+-- Init.
+box.schema.user.grant('guest', 'replication')
+_ = box.schema.space.create('glob')
+_ = box.schema.space.create('loc', {is_local=true})
+_ = box.space.glob:create_index('pk')
+_ = box.space.loc:create_index('pk')
+
+function gen_mixed_tx(i)\
+ box.begin()\
+ if bit.band(i, 1) ~= 0 then\
+ box.space.glob:insert{10 * i + 1}\
+ else\
+ box.space.loc:insert{10 * i + 1}\
+ end\
+ if bit.band(i, 2) ~= 0 then\
+ box.space.glob:insert{10 * i + 2}\
+ else\
+ box.space.loc:insert{10 * i + 2}\
+ end\
+ if bit.band(i, 4) ~= 0 then\
+ box.space.glob:insert{10 * i + 3}\
+ else\
+ box.space.loc:insert{10 * i + 3}\
+ end\
+ box.commit()\
+end
+
+test_run:cmd("create server replica with rpl_master=default,\
+ script='replication/replica.lua'")
+test_run:cmd('start server replica')
+test_run:wait_downstream(2, {status='follow'})
+
+for i = 0, 7 do gen_mixed_tx(i) end
+
+box.info.replication[2].status
+
+vclock = box.info.vclock
+vclock[0] = nil
+test_run:wait_vclock("replica", vclock)
+
+test_run:cmd('switch replica')
+
+box.info.status
+box.info.replication[1].upstream.status
+
+box.space.glob:select{}
+
+test_run:cmd('switch default')
+
+-- Cleanup.
+test_run:cmd('stop server replica')
+test_run:cmd('delete server replica')
+box.schema.user.revoke('guest', 'replication')
+box.space.loc:drop()
+box.space.glob:drop()
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index d2743b5edd..f357b07da5 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -18,6 +18,7 @@
"gh-4606-admin-creds.test.lua": {},
"gh-4739-vclock-assert.test.lua": {},
"gh-4730-applier-rollback.test.lua": {},
+ "gh-4928-tx-boundaries.test.lua": {},
"*": {
"memtx": {"engine": "memtx"},
"vinyl": {"engine": "vinyl"}
--
GitLab