Commit Diff


commit - 3f9ac0b7c9ed8dcea5fe60f57bf5580852385e58
commit + afe090767212fed32cef1203390a56ae6436e390
blob - /dev/null
blob + d18048e147f26955df8b65d7ef49d84dd3f61d8f (mode 644)
--- /dev/null
+++ changelogs/unreleased/gh-10375-vy-do-not-abort-unrelated-tx-on-ddl.md
@@ -0,0 +1,3 @@
+## bugfix/vinyl
+
+* Fixed a bug when any DDL operation aborted unrelated transactions (gh-10375).
blob - bd7d10252615b1d750923515dfbbce674d81bf0c
blob + 85febfb204075f4cffbb12faff0f31822b755f87
--- src/box/engine.h
+++ src/box/engine.h
@@ -263,6 +263,12 @@ enum {
 	 * Set if the engine supports creation of a read view.
 	 */
 	ENGINE_SUPPORTS_READ_VIEW = 1 << 1,
+	/**
+	 * Set if the engine's transaction manager properly handles
+	 * concurrent DDL operations. A DDL operation will abort all
+	 * transactions for engines that don't have this flag set.
+	 */
+	ENGINE_TXM_HANDLES_DDL = 1 << 5,
 };
 
 struct engine {
blob - f0886880f713cda367c840a6a1062fc17ae2de93
blob + 3082147aeebf26090f6ec87521e612a7a58d6e80
--- src/box/memtx_tx.c
+++ src/box/memtx_tx.c
@@ -688,6 +688,8 @@ memtx_tx_abort_all_for_ddl(struct txn *ddl_owner)
 	struct txn *to_be_aborted;
 	rlist_foreach_entry(to_be_aborted, &txm.all_txs, in_all_txs) {
 		if (to_be_aborted == ddl_owner)
+			continue;
+		if (txn_has_flag(to_be_aborted, TXN_HANDLES_DDL))
 			continue;
 		if (to_be_aborted->status != TXN_INPROGRESS &&
 		    to_be_aborted->status != TXN_IN_READ_VIEW)
blob - a85ab111ea077d8a9617b74d3c949822778db28b
blob + ec42ce5f92613b869f91c0b9b8c8efeaefbf67b6
--- src/box/txn.c
+++ src/box/txn.c
@@ -509,6 +509,11 @@ txn_begin(void)
 	 * if they are not supported.
 	 */
 	txn_set_flags(txn, TXN_CAN_YIELD);
+	/*
+	 * A transaction is unaffected by concurrent DDL as long as it has
+	 * no statements.
+	 */
+	txn_set_flags(txn, TXN_HANDLES_DDL);
 	memtx_tx_register_txn(txn);
 	rmean_collect(rmean_box, IPROTO_BEGIN, 1);
 	return txn;
@@ -521,7 +526,10 @@ txn_begin_in_engine(struct engine *engine, struct txn 
 		return 0;
 	if (txn->engine == NULL) {
 		txn->engine = engine;
-		return engine_begin(engine, txn);
+		if (engine_begin(engine, txn) != 0)
+			return -1;
+		if ((engine->flags & ENGINE_TXM_HANDLES_DDL) == 0)
+			txn_clear_flags(txn, TXN_HANDLES_DDL);
 	} else if (txn->engine != engine) {
 		/**
 		 * Only one engine can be used in
blob - 526453aa2e6583be453a4cd031342ff1ff009296
blob + e946f66cf5575b2494e0c18c8ff52dc2a0f4de16
--- src/box/txn.h
+++ src/box/txn.h
@@ -106,6 +106,12 @@ enum txn_flag {
 	 * rolled back at commit.
 	 */
 	TXN_IS_ABORTED_BY_TIMEOUT = 0x100,
+	/**
+	 * Transaction properly handles concurrent DDL operations.
+	 * If a transaction doesn't have this flag, it'll be aborted
+	 * by any DDL operation.
+	 */
+	TXN_HANDLES_DDL = 0x1000,
 };
 
 enum {
blob - 067e565930d91fd14377469bd443f7342ded1b44
blob + fe56d59630688630088af9ec8721ab70387ce4d7
--- src/box/vinyl.c
+++ src/box/vinyl.c
@@ -2739,6 +2739,7 @@ vinyl_engine_new(const char *dir, size_t memory,
 
 	env->base.vtab = &vinyl_engine_vtab;
 	env->base.name = "vinyl";
+	env->base.flags = ENGINE_TXM_HANDLES_DDL;
 	return &env->base;
 }
 
blob - /dev/null
blob + 329a13184d191a3a89c015cce14880115200f7a0 (mode 644)
--- /dev/null
+++ test/engine-luatest/gh_10375_ddl_does_not_abort_unrelated_transactions_test.lua
@@ -0,0 +1,65 @@
+local t = require('luatest')
+local server = require('luatest.server')
+
+local g = t.group(nil, t.helpers.matrix{engine = {'memtx', 'vinyl'}})
+
+g.before_all(function(cg)
+    cg.server = server:new({
+        box_cfg = {memtx_use_mvcc_engine = true},
+    })
+    cg.server:start()
+end)
+
+g.after_all(function(cg)
+    cg.server:drop()
+end)
+
+g.after_each(function(cg)
+    cg.server:exec(function()
+        if box.space.test1 ~= nil then
+            box.space.test1:drop()
+        end
+        if box.space.test2 ~= nil then
+            box.space.test2:drop()
+        end
+    end)
+end)
+
+g.test_ddl_does_not_abort_unrelated_transactions = function(cg)
+    t.skip_if(cg.params.engine == 'memtx', 'gh-10377')
+    cg.server:exec(function(engine)
+        local fiber = require('fiber')
+        box.schema.create_space('test1', {engine = engine})
+        box.space.test1:create_index('primary')
+        box.begin()
+        box.space.test1:insert({1, 10})
+        local f = fiber.new(function()
+            box.schema.create_space('test2', {engine = engine})
+            box.space.test2:create_index('primary')
+        end)
+        f:set_joinable(true)
+        t.assert_equals({f:join()}, {true})
+        t.assert_equals({pcall(box.commit)}, {true})
+        t.assert_equals(box.space.test1:select(), {{1, 10}})
+    end, {cg.params.engine})
+end
+
+g.test_ddl_aborts_related_transactions = function(cg)
+    cg.server:exec(function(engine)
+        local fiber = require('fiber')
+        box.schema.create_space('test1', {engine = engine})
+        box.space.test1:create_index('primary')
+        box.begin()
+        box.space.test1:insert({1, 10})
+        local f = fiber.new(function()
+            box.space.test1:create_index('secondary', {parts = {2, 'unsigned'}})
+        end)
+        f:set_joinable(true)
+        t.assert_equals({f:join()}, {true})
+        t.assert_error_covers({
+            type = 'ClientError',
+            code = box.error.TRANSACTION_CONFLICT,
+        }, box.commit)
+        t.assert_equals(box.space.test1:select(), {})
+    end, {cg.params.engine})
+end