Commit Diff


commit - 284cc37913fd528f08603942ef01b3726e38e260
commit + 6e1e1d50a703a26b4dbf4d7d4bcd4ba788d97864
blob - /dev/null
blob + d31ddff0bb1fc0014e87ba6ec58d311e8c19fba6 (mode 644)
--- /dev/null
+++ changelogs/unreleased/gh-9720-tmp-spaces-ddl-abort-applier-txs.md
@@ -0,0 +1,5 @@
+## bugfix/box
+
+* Now fully-temporary spaces DDL does not abort concurrent purely remote
+  (applier) transactions, and DDL in purely remote transactions does not abort
+  concurrent fully-temporary spaces transactions (gh-9720).
blob - c4cc9a427906e7812a5b26eec14963570aa8ec73
blob + f098fd6b477035901312572d011a15bc3fa71fa5
--- src/box/memtx_space.c
+++ src/box/memtx_space.c
@@ -1073,6 +1073,13 @@ memtx_space_check_format(struct space *space, struct t
 
 		ERROR_INJECT_YIELD(ERRINJ_CHECK_FORMAT_DELAY);
 
+		ERROR_INJECT_COUNTDOWN(ERRINJ_CHECK_FORMAT_DELAY_COUNTDOWN, {
+			struct errinj *e =
+				errinj(ERRINJ_CHECK_FORMAT_DELAY, ERRINJ_BOOL);
+			e->bparam = true;
+			ERROR_INJECT_YIELD(ERRINJ_CHECK_FORMAT_DELAY);
+		});
+
 		tuple_unref(state.cursor);
 		if (state.rc != 0) {
 			rc = -1;
blob - 7d6608cfa793a1c74307a5705f786b923b1c4961
blob + 5f8f5e079cd7cb7e53e95316ccc3dd12f47b925b
--- src/box/memtx_tx.c
+++ src/box/memtx_tx.c
@@ -681,6 +681,19 @@ memtx_tx_acquire_ddl(struct txn *tx)
 {
 	tx->is_schema_changed = true;
 	(void) txn_can_yield(tx, false);
+}
+
+/**
+ * Fully temporary and remote transactions cannot possibly conflict each other
+ * by definition, so we can filter them out when aborting transactions for DDL.
+ * Temporary space transactions are a always nop, since they never have WAL rows
+ * associated with them.
+ */
+static bool
+memtx_tx_filter_temporary_and_remote_txs(struct txn *txn1, struct txn *txn2)
+{
+	return (txn_is_fully_temporary(txn1) && txn_is_fully_remote(txn2)) ||
+	       (txn_is_fully_remote(txn1) && txn_is_fully_temporary(txn2));
 }
 
 void
@@ -695,6 +708,9 @@ memtx_tx_abort_all_for_ddl(struct txn *ddl_owner)
 		if (to_be_aborted->status != TXN_INPROGRESS &&
 		    to_be_aborted->status != TXN_IN_READ_VIEW)
 			continue;
+		if (memtx_tx_filter_temporary_and_remote_txs(ddl_owner,
+							     to_be_aborted))
+			continue;
 		to_be_aborted->status = TXN_ABORTED;
 		txn_set_flags(to_be_aborted, TXN_IS_CONFLICTED);
 		say_warn("Transaction committing DDL (id=%lld) has aborted "
blob - 6801c4af77b4dab7b90bffa50d00699771d02875
blob + cf12f5459c5a5604190442fdf98e3a3fbd780c81
--- src/box/txn.c
+++ src/box/txn.c
@@ -663,6 +663,40 @@ txn_is_distributed(struct txn *txn)
 	 */
 	return (txn->n_new_rows > 0 && txn->n_applier_rows > 0 &&
 		txn->n_new_rows != txn->n_local_rows);
+}
+
+bool
+txn_is_fully_temporary(struct txn *txn)
+{
+	if (!txn_is_nop(txn))
+		return false;
+	struct txn_stmt *stmt;
+	stailq_foreach_entry(stmt, &txn->stmts, next) {
+		if (stmt->space != NULL &&
+		    stmt->space->def->opts.type == SPACE_TYPE_DATA_TEMPORARY)
+			return false;
+	}
+	return true;
+}
+
+bool
+txn_is_fully_remote(struct txn *txn)
+{
+	if (txn->n_new_rows != 0)
+		return false;
+	struct txn_stmt *stmt;
+	/*
+	 * Allow DDL on data-temporary spaces, since we allow only fully
+	 * temporary transactions to continue.
+	 */
+	stailq_foreach_entry(stmt, &txn->stmts, next) {
+		if (stmt->space != NULL &&
+		    space_is_data_temporary(stmt->space)) {
+			assert(stmt->row == NULL);
+			return false;
+		}
+	}
+	return true;
 }
 
 /**
blob - 4b88ecc3cd34513f4bd632ba652ca28c630221cd
blob + a88f7361b0cad59d296b9dba6b95c20baaaa9782
--- src/box/txn.h
+++ src/box/txn.h
@@ -914,6 +914,20 @@ txn_is_nop(const struct txn *txn)
 {
 	return txn->n_new_rows + txn->n_applier_rows == 0;
 }
+
+/**
+ * Check whether a transaction consists only of operations on fully temporary
+ * spaces.
+ */
+bool
+txn_is_fully_temporary(struct txn *txn);
+
+/**
+ * Check whether a transaction consists only of remote operations coming from
+ * the applier.
+ */
+bool
+txn_is_fully_remote(struct txn *txn);
 
 /**
  * Mark @a stmt as temporary by removing the associated stmt->row
blob - 3871b0e5bcd1645be2946195aa60b500d7c8677c
blob + 82f853dd9a524864329f0441f077520a568cf197
--- src/box/vinyl.c
+++ src/box/vinyl.c
@@ -1150,6 +1150,12 @@ vinyl_space_check_format(struct space *space, struct t
 		if (++loops % VY_YIELD_LOOPS == 0)
 			fiber_sleep(0);
 		ERROR_INJECT_YIELD(ERRINJ_CHECK_FORMAT_DELAY);
+		ERROR_INJECT_COUNTDOWN(ERRINJ_CHECK_FORMAT_DELAY_COUNTDOWN, {
+			struct errinj *e =
+				errinj(ERRINJ_CHECK_FORMAT_DELAY, ERRINJ_BOOL);
+			e->bparam = true;
+			ERROR_INJECT_YIELD(ERRINJ_CHECK_FORMAT_DELAY);
+		});
 		if (ctx.is_failed) {
 			diag_move(&ctx.diag, diag_get());
 			rc = -1;
blob - 78cb42491c92186a20fc220bf22da11f74a217ea
blob + 22a2ae18e69c657c9128668356d0f301380a88f3
--- src/lib/core/errinj.h
+++ src/lib/core/errinj.h
@@ -83,6 +83,7 @@ struct errinj {
 	_(ERRINJ_BUILD_INDEX_ON_ROLLBACK_ALLOC, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_BUILD_INDEX_TIMEOUT, ERRINJ_DOUBLE, {.dparam = 0}) \
 	_(ERRINJ_CHECK_FORMAT_DELAY, ERRINJ_BOOL, {.bparam = false}) \
+	_(ERRINJ_CHECK_FORMAT_DELAY_COUNTDOWN, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_COIO_SENDFILE_CHUNK, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT, {.iparam = 0}) \
blob - /dev/null
blob + cb180a9608a93e451d0356cc22285e5238e080ca (mode 644)
--- /dev/null
+++ test/replication-luatest/gh_9720_tmp_spaces_ddl_abort_applier_txs_test.lua
@@ -0,0 +1,276 @@
+local t = require('luatest')
+local replica_set = require('luatest.replica_set')
+local server = require('luatest.server')
+
+local g = t.group(nil, t.helpers.matrix({memtx_use_mvcc = {false, true}}))
+
+g.before_each(function(cg)
+    cg.replica_set = replica_set:new{}
+    local box_cfg = {
+        memtx_use_mvcc_engine = cg.params.memtx_use_mvcc,
+        replication = {
+            server.build_listen_uri('server1', cg.replica_set.id),
+            server.build_listen_uri('server2', cg.replica_set.id),
+        },
+        replication_timeout = 0.1,
+    }
+    cg.server1 = cg.replica_set:build_and_add_server{
+        alias = 'server1',
+        box_cfg = box_cfg,
+    }
+    box_cfg.read_only = true
+    cg.server2 = cg.replica_set:build_and_add_server{
+        alias = 'server2',
+        box_cfg = box_cfg,
+    }
+    cg.replica_set:start()
+    cg.server1:exec(function()
+        local s = box.schema.create_space('s')
+        s:create_index('p')
+        box.begin()
+        for i = 1, 1000 do
+            s:replace{i, i}
+        end
+        box.commit()
+    end)
+    cg.server2:exec(function()
+        box.cfg{read_only = false}
+    end)
+    cg.replica_set:wait_for_fullmesh()
+    cg.server1:wait_for_downstream_to(cg.server2)
+end)
+
+g.after_each(function(cg)
+    cg.replica_set:drop()
+end)
+
+-- Test that a DDL transaction on fully temporary spaces does not abort a remote
+-- transaction.
+g.test_fully_temporary_ddl_does_not_abort_remote_tx = function(cg)
+    t.tarantool.skip_if_not_debug()
+
+    cg.server2:exec(function()
+        box.error.injection.set('ERRINJ_CHECK_FORMAT_DELAY_COUNTDOWN', 0)
+    end)
+    cg.server1:exec(function()
+        box.space.s:format{'i', 'unsigned'}
+    end)
+    cg.server2:exec(function()
+        t.helpers.retrying({timeout = 120}, function()
+            t.assert(box.error.injection.get('ERRINJ_CHECK_FORMAT_DELAY', true))
+        end)
+        box.schema.space.create('tmp', {type = 'temporary'})
+        box.error.injection.set('ERRINJ_CHECK_FORMAT_DELAY', false)
+    end)
+    cg.server1:wait_for_downstream_to(cg.server2)
+    cg.server2:exec(function()
+        local msg = "Transaction has been aborted by conflict"
+        t.assert_not_equals(box.info.replication[1].upstream.message, msg)
+        t.assert_not_equals(box.space.s:format(), {})
+    end)
+end
+
+-- Test that a DDL transaction on fully temporary spaces still aborts a not
+-- fully remote transaction.
+g.test_fully_temporary_ddl_aborts_not_fully_remote_tx = function(cg)
+    t.skip_if(not cg.params.memtx_use_mvcc)
+
+    cg.server2:exec(function()
+        local tmp = box.schema.space.create('tmp-replace', {type = 'temporary'})
+        tmp:create_index('p')
+
+        box.space.s:on_replace(function()
+            tmp:replace{0}
+            rawset(_G, 'executed_on_replace', true)
+            require('fiber').sleep(60)
+        end)
+    end)
+    cg.server1:exec(function()
+        box.space.s:replace{0}
+    end)
+    cg.server2:exec(function()
+        t.helpers.retrying({timeout = 60}, function()
+            t.assert(_G.executed_on_replace)
+        end)
+        box.schema.space.create('tmp', {type = 'temporary'})
+    end)
+    local msg = "Transaction committing DDL %(id=%d+%) has aborted " ..
+                "another TX %(id=%d+%)"
+    t.assert(cg.server2:grep_log(msg))
+    cg.server2:exec(function()
+        t.assert_equals(box.space.s:get{0}, nil)
+    end)
+end
+
+-- Test that a DDL transaction on fully temporary spaces does not abort a
+-- fully remote transaction that rollbacked to a savepoint without local
+-- changes.
+g.test_fully_temporary_ddl_does_not_abort_fully_remote_tx_after_rb_to_svp =
+function(cg)
+    t.skip_if(not cg.params.memtx_use_mvcc)
+
+    cg.server2:exec(function()
+        local tmp = box.schema.space.create('tmp-replace', {type = 'temporary'})
+        tmp:create_index('p')
+
+        box.space.s:on_replace(function()
+            local svp = box.savepoint()
+            tmp:replace{0}
+            box.rollback_to_savepoint(svp)
+            rawset(_G, 'executed_on_replace', true)
+            t.helpers.retrying({timeout = 120}, function()
+                t.assert(_G.can_leave_on_replace)
+            end)
+        end)
+    end)
+    cg.server1:exec(function()
+        box.space.s:replace{0}
+    end)
+    cg.server2:exec(function()
+        t.helpers.retrying({timeout = 120}, function()
+            t.assert(_G.executed_on_replace)
+        end)
+        box.schema.space.create('tmp', {type = 'temporary'})
+        rawset(_G, 'can_leave_on_replace', true)
+    end)
+    cg.server1:wait_for_downstream_to(cg.server2)
+    cg.server2:exec(function()
+        t.assert_not_equals(box.space.s:get{0}, nil)
+    end)
+end
+
+-- Test that a remote DDL transaction does not abort a transaction on fully
+-- temporary spaces.
+g.test_fully_remote_ddl_does_not_abort_fully_temporary_tx = function(cg)
+    t.tarantool.skip_if_not_debug()
+
+    local fid = cg.server2:exec(function()
+        local tmp = box.schema.space.create('tmp', {type = 'temporary'})
+        tmp:create_index('p')
+        box.begin()
+        for i = 1, 1000 do
+            tmp:replace{i, i}
+        end
+        box.commit()
+        box.error.injection.set('ERRINJ_CHECK_FORMAT_DELAY', true)
+        local f = require('fiber').new(function()
+            tmp:format{'i', 'unsigned'}
+        end)
+        f:set_joinable(true)
+        return f:id()
+    end)
+    cg.server1:exec(function()
+        box.schema.space.create('ss')
+    end)
+    cg.server1:wait_for_downstream_to(cg.server2)
+    cg.server2:exec(function(fid)
+        box.error.injection.set('ERRINJ_CHECK_FORMAT_DELAY', false)
+        local ok = require('fiber').find(fid):join()
+        t.assert(ok)
+        t.assert_not_equals(box.space.tmp:format(), {})
+    end, {fid})
+end
+
+-- Test that a remote DDL transaction still aborts a not fully temporary
+-- transaction.
+g.test_fully_remote_tx_ddl_aborts_not_fully_temporary_tx = function(cg)
+    t.skip_if(not cg.params.memtx_use_mvcc)
+
+    local fid = cg.server2:exec(function()
+        local fiber = require('fiber')
+
+        local tmp = box.schema.space.create('tmp-replace', {type = 'temporary'})
+        tmp:create_index('p')
+
+        rawset(_G, 'downstream_cond', fiber.cond())
+        local f = fiber.new(function()
+            box.begin()
+            tmp:replace{0}
+            box.space.s:replace{0}
+            _G.downstream_cond:wait()
+            box.commit()
+        end)
+        f:set_joinable(true)
+        return f:id()
+    end)
+    cg.server1:exec(function()
+        box.schema.space.create('ss')
+    end)
+    cg.server1:wait_for_downstream_to(cg.server2)
+    cg.server2:exec(function(fid)
+        _G.downstream_cond:signal()
+        local ok, err = require('fiber').find(fid):join()
+        t.assert_not(ok)
+        t.assert_equals(err.message, "Transaction has been aborted by conflict")
+    end, {fid})
+end
+
+-- Test that a remote DDL transaction does not abort a fully temporary
+-- transaction that rolled back to a savepoint without non-temporary changes.
+g.test_fully_remote_tx_ddl_does_not_abort_fully_temporary_tx_after_rb_to_svp =
+function(cg)
+    t.skip_if(not cg.params.memtx_use_mvcc)
+
+    local fid = cg.server2:exec(function()
+        local fiber = require('fiber')
+
+        local tmp = box.schema.space.create('tmp-replace', {type = 'temporary'})
+        tmp:create_index('p')
+
+        rawset(_G, 'downstream_cond', fiber.cond())
+        local f = fiber.new(function()
+            box.begin()
+            tmp:replace{0}
+            local svp = box.savepoint()
+            box.space.s:replace{0}
+            box.rollback_to_savepoint(svp)
+            _G.downstream_cond:wait()
+            box.commit()
+        end)
+        f:set_joinable(true)
+        return f:id()
+    end)
+    cg.server1:exec(function()
+        box.schema.space.create('ss')
+    end)
+    cg.server1:wait_for_downstream_to(cg.server2)
+    cg.server2:exec(function(fid)
+        _G.downstream_cond:signal()
+        local ok = require('fiber').find(fid):join()
+        t.assert(ok)
+    end, {fid})
+end
+
+-- Test that a fully remote DDL transaction still aborts a data-temporary
+-- transaction.
+g.test_fully_remote_tx_ddl_aborts_data_temporary_tx = function(cg)
+    t.skip_if(not cg.params.memtx_use_mvcc)
+
+    local fid = cg.server2:exec(function()
+        local fiber = require('fiber')
+
+        local tmp = box.schema.space.create('data-tmp',
+                                            {type = 'data-temporary'})
+        tmp:create_index('p')
+
+        rawset(_G, 'downstream_cond', fiber.cond())
+        local f = fiber.new(function()
+            box.begin()
+            tmp:replace{0}
+            _G.downstream_cond:wait()
+            box.commit()
+        end)
+        f:set_joinable(true)
+        return f:id()
+    end)
+    cg.server1:exec(function()
+        box.schema.space.create('ss')
+    end)
+    cg.server1:wait_for_downstream_to(cg.server2)
+    cg.server2:exec(function(fid)
+        _G.downstream_cond:signal()
+        local ok, err = require('fiber').find(fid):join()
+        t.assert_not(ok)
+        t.assert_equals(err.message, "Transaction has been aborted by conflict")
+    end, {fid})
+end