[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