[patches] [PATCH] Don't try to lock a ddl latch in a multistatement tx

Georgy Kirichenko georgy at tarantool.org
Thu Mar 1 17:12:40 MSK 2018


Any ddl is prohibited in a multistatement transaction, there is no
reason to try to lock a ddl latch in tis case. Locking for already
locked latch will cause an yield and a silent transaction rollback, and
this will crash or assert tarantool server.

Fixes #2783
---
https://github.com/tarantool/tarantool/tree/gh-2783-check-multistatement-tx-for-ddl-lock

src/box/alter.cc            |  7 +++++++
 test/box/ddl.result         | 42 ++++++++++++++++++++++++++++++++++++++++++
 test/box/ddl.test.lua       | 23 +++++++++++++++++++++++
 test/box/on_replace.result  |  2 +-
 test/box/transaction.result |  2 +-
 test/engine/truncate.result |  2 +-
 6 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 5749740d2..b94cf710b 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3133,6 +3133,13 @@ lock_before_dd(struct trigger *trigger, void *event)
 	if (fiber() == latch_owner(&schema_lock))
 		return;
 	struct txn *txn = (struct txn *)event;
+	/*
+	 * The trigger is executed before any check and may yield but
+	 * an yield in a non-autocommit memtx transaction will rollback it.
+	 * So we shouldn't to try to lock a latch if there is a multistatement
+	 * transaction.
+	 */
+	txn_check_singlestatement_xc(txn, "DDL");
 	latch_lock(&schema_lock);
 	struct trigger *on_commit =
 		txn_alter_trigger_new(unlock_after_dd, NULL);
diff --git a/test/box/ddl.result b/test/box/ddl.result
index 0eef37992..145f6bd10 100644
--- a/test/box/ddl.result
+++ b/test/box/ddl.result
@@ -514,3 +514,45 @@ s:format()[3].custom_field
 s:drop()
 ---
 ...
+-- issue 2783
+-- A ddl operation shoud fail before trying to lock a ddl latch bacause
+-- if a ddl operation tries to lock already locked latch then current 
+-- transaction will be silently rollbacked under our feet
+test_latch = box.schema.space.create('test_latch')
+---
+...
+_ = test_latch:create_index('primary', {unique = true, parts = {1, 'unsigned'}})
+---
+...
+fiber = require('fiber')
+---
+...
+c = fiber.channel(1)
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+_ = fiber.create(function()
+    test_latch:create_index("sec", {unique = true, parts = {2, 'unsigned'}})
+    c:put(true)
+end) ;
+---
+...
+box.begin()
+test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}})
+box.commit() ;
+---
+- error: DDL does not support multi-statement transactions
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+_ = c:get()
+---
+...
+test_latch:drop() -- this is where everything stops
+---
+...
diff --git a/test/box/ddl.test.lua b/test/box/ddl.test.lua
index 820fe7d4d..67601aafd 100644
--- a/test/box/ddl.test.lua
+++ b/test/box/ddl.test.lua
@@ -198,3 +198,26 @@ format[3] = {'field3', 'unsigned', custom_field = 'custom_value'}
 s = box.schema.create_space('test', {format = format})
 s:format()[3].custom_field
 s:drop()
+
+-- issue 2783
+-- A ddl operation shoud fail before trying to lock a ddl latch bacause
+-- if a ddl operation tries to lock already locked latch then current 
+-- transaction will be silently rollbacked under our feet
+test_latch = box.schema.space.create('test_latch')
+_ = test_latch:create_index('primary', {unique = true, parts = {1, 'unsigned'}})
+fiber = require('fiber')
+c = fiber.channel(1)
+test_run:cmd("setopt delimiter ';'")
+_ = fiber.create(function()
+    test_latch:create_index("sec", {unique = true, parts = {2, 'unsigned'}})
+    c:put(true)
+end) ;
+
+box.begin()
+test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}})
+box.commit() ;
+
+test_run:cmd("setopt delimiter ''");
+
+_ = c:get()
+test_latch:drop() -- this is where everything stops
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index d6158dc5a..26d074a94 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -492,7 +492,7 @@ t = s:on_replace(function () s:drop() end, t)
 ...
 s:replace({5, 6})
 ---
-- error: Space _index does not support multi-statement transactions
+- error: DDL does not support multi-statement transactions
 ...
 t = s:on_replace(function () box.schema.func.create('newf') end, t)
 ---
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 2a4b3b289..f60846e75 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -107,7 +107,7 @@ s = box.schema.space.create('test');
 ...
 box.begin() index = s:create_index('primary');
 ---
-- error: Space _index does not support multi-statement transactions
+- error: DDL does not support multi-statement transactions
 ...
 box.rollback();
 ---
diff --git a/test/engine/truncate.result b/test/engine/truncate.result
index b6e1a99a2..7f294031f 100644
--- a/test/engine/truncate.result
+++ b/test/engine/truncate.result
@@ -24,7 +24,7 @@ box.begin()
 ...
 s:truncate()
 ---
-- error: Space _truncate does not support multi-statement transactions
+- error: DDL does not support multi-statement transactions
 ...
 box.commit()
 ---
-- 
2.16.2




More information about the Tarantool-patches mailing list