diff --git a/src/box/alter.cc b/src/box/alter.cc index 0e611400c11481a23d6996a41de532ea5cc33106..5dec2519daa6559cea68382eb1928f7db08b563f 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -3153,6 +3153,15 @@ lock_before_dd(struct trigger *trigger, void *event) if (fiber() == latch_owner(&schema_lock)) return; struct txn *txn = (struct txn *)event; + /* + * This trigger is executed before any check and may yield + * on the latch lock. But a yield in a non-autocommit + * memtx transaction will roll it back silently, rather + * than produce an error, which is very confusing. + * So don't 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 0eef379928b20e2e40a0491813f9a1870cb9e879..f6991840f7089d45c7d0ef6a4204ed4aa13861dc 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -514,3 +514,50 @@ s:format()[3].custom_field s:drop() --- ... +-- +-- gh-2783 +-- A ddl operation shoud fail before trying to lock a ddl latch +-- in a multi-statement transaction. +-- If operation tries to lock already an locked latch then the +-- current transaction will be silently rolled back under our feet. +-- This is confusing. So check for multi-statement transaction +-- before locking the latch. +-- +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 820fe7d4dc66bdb3fb0ea103666c827d5e2d5f71..6184ccb1a333e7f5c96feb73b90fb1ec7560e250 100644 --- a/test/box/ddl.test.lua +++ b/test/box/ddl.test.lua @@ -198,3 +198,31 @@ format[3] = {'field3', 'unsigned', custom_field = 'custom_value'} s = box.schema.create_space('test', {format = format}) s:format()[3].custom_field s:drop() + +-- +-- gh-2783 +-- A ddl operation shoud fail before trying to lock a ddl latch +-- in a multi-statement transaction. +-- If operation tries to lock already an locked latch then the +-- current transaction will be silently rolled back under our feet. +-- This is confusing. So check for multi-statement transaction +-- before locking the latch. +-- +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 d6158dc5a10e08f8b0806802779948924da8fd41..26d074a946e7e16ec29a7c8691e16d0aff6741ef 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 2a4b3b289b4d833da65883db35a5167a744d7d5e..f60846e75ab11547043d8c964076ee027c337286 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 b6e1a99a28a62690a301ebd5ed3773f5e22dc929..7f294031ff906d7aa3d39ac643526f6736ca5d66 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() ---