Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH 0/3] Enable ddl between begin and commit statements
@ 2019-03-20 20:31 Georgy Kirichenko
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback Georgy Kirichenko
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Georgy Kirichenko @ 2019-03-20 20:31 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

This patchset consist of three commits that are making possible a ddl
between begin and commit.

* The first patch fixes truncate rollback behavior because of vinyl's
  internal assertion.
* The second one synchronizes lua schema updates with the space cache
* The third one brings a possibility of a ddl operation in a scope of
  an explicitly started transaction.

Branch: https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-2798-ddl-between-begin-commit
Issue: https://github.com/tarantool/tarantool/issues/2798

Georgy Kirichenko (3):
  Abort vinyl index creation in case of truncation rollback
  Synchronize lua schema update with space cache
  Require for single statement not autocommit in case of ddl

 src/box/alter.cc               | 21 ++++++--
 src/box/memtx_engine.c         | 20 ++------
 src/box/txn.c                  |  9 +++-
 src/box/txn.h                  |  5 ++
 test/box/ddl.result            |  8 +--
 test/box/ddl.test.lua          |  4 +-
 test/box/transaction.result    | 53 +++++---------------
 test/box/transaction.test.lua  | 27 ++++------
 test/engine/ddl.result         | 92 ++++++++++++++++++++++++++++++++++
 test/engine/ddl.test.lua       | 55 ++++++++++++++++++++
 test/engine/errinj.result      | 34 +++++++++++++
 test/engine/errinj.test.lua    | 12 +++++
 test/engine/suite.ini          |  1 +
 test/engine/truncate.result    |  3 +-
 test/sql-tap/trigger1.test.lua | 12 ++---
 test/sql/delete.result         |  8 ++-
 test/sql/delete.test.lua       |  3 +-
 17 files changed, 267 insertions(+), 100 deletions(-)
 create mode 100644 test/engine/errinj.result
 create mode 100644 test/engine/errinj.test.lua

-- 
2.21.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback
  2019-03-20 20:31 [tarantool-patches] [PATCH 0/3] Enable ddl between begin and commit statements Georgy Kirichenko
@ 2019-03-20 20:31 ` Georgy Kirichenko
  2019-03-21 20:16   ` Vladimir Davydov
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 2/3] Synchronize lua schema update with space cache Georgy Kirichenko
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 3/3] Require for single statement not autocommit in case of ddl Georgy Kirichenko
  2 siblings, 1 reply; 5+ messages in thread
From: Georgy Kirichenko @ 2019-03-20 20:31 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

Abort index create if truncate couldn't be finished because of rollback
or error. Without this vinyl will fail because of internal scheduler
assertion.

Needed for: 2798
---
 src/box/alter.cc            |  8 ++++++++
 test/engine/errinj.result   | 34 ++++++++++++++++++++++++++++++++++
 test/engine/errinj.test.lua | 12 ++++++++++++
 test/engine/suite.ini       |  1 +
 4 files changed, 55 insertions(+)
 create mode 100644 test/engine/errinj.result
 create mode 100644 test/engine/errinj.test.lua

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 431da12da..84e74ee89 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1312,6 +1312,7 @@ public:
 	uint32_t iid;
 	virtual void prepare(struct alter_space *alter);
 	virtual void commit(struct alter_space *alter, int64_t signature);
+	virtual void rollback(struct alter_space *alter);
 };
 
 void
@@ -1349,6 +1350,13 @@ TruncateIndex::commit(struct alter_space *alter, int64_t signature)
 	index_commit_create(new_index, signature);
 }
 
+void
+TruncateIndex::rollback(struct alter_space *alter)
+{
+	struct index *new_index = space_index(alter->new_space, iid);
+	index_abort_create(new_index);
+}
+
 /**
  * UpdateSchemaVersion - increment schema_version. Used on
  * in alter_space_do(), i.e. when creating or dropping
diff --git a/test/engine/errinj.result b/test/engine/errinj.result
new file mode 100644
index 000000000..37fc280b9
--- /dev/null
+++ b/test/engine/errinj.result
@@ -0,0 +1,34 @@
+test_run = require('test_run')
+---
+...
+inspector = test_run.new()
+---
+...
+engine = inspector:get_cfg('engine')
+---
+...
+errinj = box.error.injection
+---
+...
+-- test truncate rollback does not abort
+s = box.schema.space.create('truncate_rollback', {engine = engine})
+---
+...
+_ = s:create_index('pk')
+---
+...
+errinj.set('ERRINJ_WAL_IO', true)
+---
+- ok
+...
+s:truncate()
+---
+- error: Failed to write to disk
+...
+errinj.set('ERRINJ_WAL_IO', false)
+---
+- ok
+...
+s:drop()
+---
+...
diff --git a/test/engine/errinj.test.lua b/test/engine/errinj.test.lua
new file mode 100644
index 000000000..ee5e36361
--- /dev/null
+++ b/test/engine/errinj.test.lua
@@ -0,0 +1,12 @@
+test_run = require('test_run')
+inspector = test_run.new()
+engine = inspector:get_cfg('engine')
+errinj = box.error.injection
+
+-- test truncate rollback does not abort
+s = box.schema.space.create('truncate_rollback', {engine = engine})
+_ = s:create_index('pk')
+errinj.set('ERRINJ_WAL_IO', true)
+s:truncate()
+errinj.set('ERRINJ_WAL_IO', false)
+s:drop()
diff --git a/test/engine/suite.ini b/test/engine/suite.ini
index 3f82a1325..3db02ab6f 100644
--- a/test/engine/suite.ini
+++ b/test/engine/suite.ini
@@ -3,6 +3,7 @@ core = tarantool
 description = tarantool multiengine tests
 script = box.lua
 use_unix_sockets = True
+release_disabled = errinj.test.lua
 config = engine.cfg
 #disabled = replica_join.test.lua
 lua_libs = conflict.lua ../box/lua/utils.lua ../box/lua/push.lua
-- 
2.21.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] [PATCH 2/3] Synchronize lua schema update with space cache
  2019-03-20 20:31 [tarantool-patches] [PATCH 0/3] Enable ddl between begin and commit statements Georgy Kirichenko
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback Georgy Kirichenko
@ 2019-03-20 20:31 ` Georgy Kirichenko
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 3/3] Require for single statement not autocommit in case of ddl Georgy Kirichenko
  2 siblings, 0 replies; 5+ messages in thread
From: Georgy Kirichenko @ 2019-03-20 20:31 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

Update lua schema as soon as space cache replace was done instead of
doing this while on_commit trigger executes. In opposite then case
schema changes would not be visible until commit was finished.

Needed for: #2798
---
 src/box/alter.cc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 84e74ee89..d4cc5c0cc 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -750,8 +750,6 @@ alter_space_commit(struct trigger *trigger, void *event)
 		op->commit(alter, txn->signature);
 	}
 
-	trigger_run_xc(&on_alter_space, alter->new_space);
-
 	alter->new_space = NULL; /* for alter_space_delete(). */
 	/*
 	 * Delete the old version of the space, we are not
@@ -787,6 +785,8 @@ alter_space_rollback(struct trigger *trigger, void * /* event */)
 	space_swap_triggers(alter->new_space, alter->old_space);
 	space_swap_fk_constraints(alter->new_space, alter->old_space);
 	space_cache_replace(alter->new_space, alter->old_space);
+	trigger_run(&on_alter_space, alter->old_space);
+
 	alter_space_delete(alter);
 }
 
@@ -888,6 +888,7 @@ alter_space_do(struct txn *txn, struct alter_space *alter)
 	 * cache with it.
 	 */
 	space_cache_replace(alter->old_space, alter->new_space);
+	trigger_run_xc(&on_alter_space, alter->new_space);
 
 	/*
 	 * Install transaction commit/rollback triggers to either
@@ -1387,7 +1388,6 @@ on_drop_space_commit(struct trigger *trigger, void *event)
 {
 	(void) event;
 	struct space *space = (struct space *)trigger->data;
-	trigger_run_xc(&on_alter_space, space);
 	space_delete(space);
 }
 
@@ -1402,6 +1402,7 @@ on_drop_space_rollback(struct trigger *trigger, void *event)
 	(void) event;
 	struct space *space = (struct space *)trigger->data;
 	space_cache_replace(NULL, space);
+	trigger_run(&on_alter_space, space);
 }
 
 /**
@@ -1411,8 +1412,7 @@ static void
 on_create_space_commit(struct trigger *trigger, void *event)
 {
 	(void) event;
-	struct space *space = (struct space *)trigger->data;
-	trigger_run_xc(&on_alter_space, space);
+	(void) trigger;
 }
 
 /**
@@ -1428,6 +1428,7 @@ on_create_space_rollback(struct trigger *trigger, void *event)
 	(void) event;
 	struct space *space = (struct space *)trigger->data;
 	space_cache_replace(space, NULL);
+	trigger_run(&on_alter_space, space);
 	space_delete(space);
 }
 
@@ -1677,6 +1678,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 		 * execution on a replica.
 		 */
 		space_cache_replace(NULL, space);
+		trigger_run_xc(&on_alter_space, space);
 		/*
 		 * Do not forget to update schema_version right after
 		 * inserting the space to the space_cache, since no
@@ -1769,6 +1771,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 		 * execution on a replica.
 		 */
 		space_cache_replace(old_space, NULL);
+		trigger_run_xc(&on_alter_space, old_space);
 		/*
 		 * Do not forget to update schema_version right after
 		 * deleting the space from the space_cache, since no
-- 
2.21.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] [PATCH 3/3] Require for single statement not autocommit in case of ddl
  2019-03-20 20:31 [tarantool-patches] [PATCH 0/3] Enable ddl between begin and commit statements Georgy Kirichenko
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback Georgy Kirichenko
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 2/3] Synchronize lua schema update with space cache Georgy Kirichenko
@ 2019-03-20 20:31 ` Georgy Kirichenko
  2 siblings, 0 replies; 5+ messages in thread
From: Georgy Kirichenko @ 2019-03-20 20:31 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

Allow single statement transactions within begin/commit in case of an
ddl operation instead of auto commit requirements. This is essential
for a transactional applier.

Needed for: #2798
---
 src/box/memtx_engine.c         | 20 ++------
 src/box/txn.c                  |  9 +++-
 src/box/txn.h                  |  5 ++
 test/box/ddl.result            |  8 +--
 test/box/ddl.test.lua          |  4 +-
 test/box/transaction.result    | 53 +++++---------------
 test/box/transaction.test.lua  | 27 ++++------
 test/engine/ddl.result         | 92 ++++++++++++++++++++++++++++++++++
 test/engine/ddl.test.lua       | 55 ++++++++++++++++++++
 test/engine/truncate.result    |  3 +-
 test/sql-tap/trigger1.test.lua | 12 ++---
 test/sql/delete.result         |  8 ++-
 test/sql/delete.test.lua       |  3 +-
 13 files changed, 204 insertions(+), 95 deletions(-)

diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index d468d1cd8..924f8bbc4 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -384,15 +384,7 @@ static int
 memtx_engine_begin(struct engine *engine, struct txn *txn)
 {
 	(void)engine;
-	/*
-	 * Register a trigger to rollback transaction on yield.
-	 * This must be done in begin(), since it's
-	 * the first thing txn invokes after txn->n_stmts++,
-	 * to match with trigger_clear() in rollbackStatement().
-	 */
-	if (txn->is_autocommit == false) {
-		memtx_init_txn(txn);
-	}
+	(void)txn;
 	return 0;
 }
 
@@ -404,15 +396,9 @@ memtx_engine_begin_statement(struct engine *engine, struct txn *txn)
 	if (txn->engine_tx == NULL) {
 		struct space *space = txn_last_stmt(txn)->space;
 
-		if (space->def->id > BOX_SYSTEM_ID_MAX &&
-		    ! rlist_empty(&space->on_replace)) {
-			/**
-			 * A space on_replace trigger may initiate
-			 * a yield.
-			 */
-			assert(txn->is_autocommit);
+		if (space->def->id > BOX_SYSTEM_ID_MAX)
+			/* Setup triggers for non-ddl transactions. */
 			memtx_init_txn(txn);
-		}
 	}
 	return 0;
 }
diff --git a/src/box/txn.c b/src/box/txn.c
index deb4fac47..ad77dd76c 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -147,6 +147,7 @@ txn_begin(bool is_autocommit)
 	txn->n_local_rows = 0;
 	txn->n_remote_rows = 0;
 	txn->is_autocommit = is_autocommit;
+	txn->is_single_statement = false;
 	txn->has_triggers  = false;
 	txn->is_aborted = false;
 	txn->in_sub_stmt = 0;
@@ -191,6 +192,11 @@ txn_begin_stmt(struct space *space)
 		diag_set(ClientError, ER_SUB_STMT_MAX);
 		return NULL;
 	}
+	if (!stailq_empty(&txn->stmts) && txn->is_single_statement) {
+		diag_set(ClientError, ER_UNSUPPORTED,
+			 "DDL", "multi-statement transactions");
+		return NULL;
+	}
 
 	struct txn_stmt *stmt = txn_stmt_new(txn);
 	if (stmt == NULL) {
@@ -430,11 +436,12 @@ txn_abort(struct txn *txn)
 int
 txn_check_singlestatement(struct txn *txn, const char *where)
 {
-	if (!txn->is_autocommit || !txn_is_first_statement(txn)) {
+	if (!txn_is_first_statement(txn)) {
 		diag_set(ClientError, ER_UNSUPPORTED,
 			 where, "multi-statement transactions");
 		return -1;
 	}
+	txn->is_single_statement = true;
 	return 0;
 }
 
diff --git a/src/box/txn.h b/src/box/txn.h
index c9829da9e..3572b005d 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -149,6 +149,11 @@ struct txn {
 	 * (statement end causes an automatic transaction commit).
 	 */
 	bool is_autocommit;
+	/**
+	 * True if this transaction is allowed to have only one statement.
+	 * Used for ddl operations.
+	 */
+	bool is_single_statement;
 	/**
 	 * True if the transaction was aborted so should be
 	 * rolled back at commit.
diff --git a/test/box/ddl.result b/test/box/ddl.result
index 3d6d07f43..e48284ffd 100644
--- a/test/box/ddl.result
+++ b/test/box/ddl.result
@@ -299,11 +299,7 @@ box.space._collation:replace(c)
 ---
 - error: collation does not support alter
 ...
-box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU')
----
-- error: Space _collation does not support multi-statement transactions
-...
-box.rollback()
+box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU') box.rollback()
 ---
 ...
 box.internal.collation.create('test', 'ICU', 'ru_RU')
@@ -645,11 +641,11 @@ _ = fiber.create(function()
 end);
 ---
 ...
+-- Should be Ok for now
 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 ''");
 ---
diff --git a/test/box/ddl.test.lua b/test/box/ddl.test.lua
index 5c147cdfb..101bc6f9b 100644
--- a/test/box/ddl.test.lua
+++ b/test/box/ddl.test.lua
@@ -131,8 +131,7 @@ c = box.space._collation:get{1}:totable()
 c[2] = 'unicode_test'
 box.space._collation:replace(c)
 
-box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU')
-box.rollback()
+box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU') box.rollback()
 
 box.internal.collation.create('test', 'ICU', 'ru_RU')
 box.internal.collation.exists('test')
@@ -260,6 +259,7 @@ _ = fiber.create(function()
     c:put(true)
 end);
 
+-- Should be Ok for now
 box.begin()
     test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}})
 box.commit();
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 8a4d11d3b..7def44d5d 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -84,77 +84,50 @@ while f:status() ~= 'dead' do fiber.sleep(0) end;
 ---
 ...
 -- transactions and system spaces
+-- some operation involves more than one ddl spaces, so they should fail
 box.begin() box.schema.space.create('test');
 ---
-- error: Space _schema does not support multi-statement transactions
-...
-box.rollback();
----
-...
-box.begin() box.schema.func.create('test');
----
-- error: Space _func does not support multi-statement transactions
+- error: DDL does not support multi-statement transactions
 ...
 box.rollback();
 ---
 ...
 box.begin() box.schema.user.create('test');
 ---
-- error: Space _user does not support multi-statement transactions
-...
-box.rollback();
----
-...
-box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv');
----
-- error: Space _priv does not support multi-statement transactions
-...
-box.rollback();
----
-...
-box.begin() box.space._user:delete{box.schema.GUEST_ID};
----
-- error: Space _user does not support multi-statement transactions
+- error: DDL does not support multi-statement transactions
 ...
 box.rollback();
 ---
 ...
-box.begin() box.space._space:delete{box.schema.CLUSTER_ID};
+-- but this is Ok now
+box.begin() box.schema.func.create('test') box.rollback();
 ---
-- error: DDL does not support multi-statement transactions
 ...
-box.rollback();
+box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv') box.rollback();
 ---
 ...
-box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false};
+space = box.schema.space.create('test');
 ---
-- error: Space _sequence does not support multi-statement transactions
 ...
-box.rollback();
+box.begin() box.space._space:delete{space.id} box.rollback();
 ---
 ...
-box.begin() box.space._schema:insert{'test'};
+box.begin() box.space._space:delete{space.id} box.commit();
 ---
-- error: Space _schema does not support multi-statement transactions
 ...
-box.rollback();
+box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false} box.rollback();
 ---
 ...
-box.begin() box.space._cluster:insert{123456789, 'abc'};
+box.begin() box.space._schema:insert{'test'} box.rollback();
 ---
-- error: Space _cluster does not support multi-statement transactions
 ...
-box.rollback();
+box.begin() box.space._cluster:insert{30, '00000000-0000-0000-0000-000000000001'} box.rollback();
 ---
 ...
 s = box.schema.space.create('test');
 ---
 ...
-box.begin() index = s:create_index('primary');
----
-- error: DDL does not support multi-statement transactions
-...
-box.rollback();
+box.begin() index = s:create_index('primary') box.rollback();
 ---
 ...
 index = s:create_index('primary');
diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua
index 8f7dfedec..0d212ca29 100644
--- a/test/box/transaction.test.lua
+++ b/test/box/transaction.test.lua
@@ -41,27 +41,22 @@ f = fiber.create(sloppy);
 -- ensure it's rolled back automatically
 while f:status() ~= 'dead' do fiber.sleep(0) end;
 -- transactions and system spaces
+-- some operation involves more than one ddl spaces, so they should fail
 box.begin() box.schema.space.create('test');
 box.rollback();
-box.begin() box.schema.func.create('test');
-box.rollback();
 box.begin() box.schema.user.create('test');
 box.rollback();
-box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv');
-box.rollback();
-box.begin() box.space._user:delete{box.schema.GUEST_ID};
-box.rollback();
-box.begin() box.space._space:delete{box.schema.CLUSTER_ID};
-box.rollback();
-box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false};
-box.rollback();
-box.begin() box.space._schema:insert{'test'};
-box.rollback();
-box.begin() box.space._cluster:insert{123456789, 'abc'};
-box.rollback();
+-- but this is Ok now
+box.begin() box.schema.func.create('test') box.rollback();
+box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv') box.rollback();
+space = box.schema.space.create('test');
+box.begin() box.space._space:delete{space.id} box.rollback();
+box.begin() box.space._space:delete{space.id} box.commit();
+box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false} box.rollback();
+box.begin() box.space._schema:insert{'test'} box.rollback();
+box.begin() box.space._cluster:insert{30, '00000000-0000-0000-0000-000000000001'} box.rollback();
 s = box.schema.space.create('test');
-box.begin() index = s:create_index('primary');
-box.rollback();
+box.begin() index = s:create_index('primary') box.rollback();
 index = s:create_index('primary');
 t = nil
 function multi()
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 8d34d5ef4..c493bd4ac 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -2075,6 +2075,15 @@ i3:select()
 ...
 -- Check that recovery works.
 inspector:cmd("restart server default")
+test_run = require('test_run')
+---
+...
+inspector = test_run.new()
+---
+...
+engine = inspector:get_cfg('engine')
+---
+...
 s = box.space.test
 ---
 ...
@@ -2130,3 +2139,86 @@ box.snapshot()
 s:drop()
 ---
 ...
+-- test ddl operation within begin/commit/rollback
+-- acquire free space id
+space = box.schema.space.create('ddl_test', {engine = engine})
+---
+...
+id = space.id
+---
+...
+space:drop()
+---
+...
+inspector:cmd("setopt delimiter ';'")
+---
+- true
+...
+box.begin()
+s = box.schema.space.create('ddl_test', {engine = engine, id = id})
+box.rollback();
+---
+...
+box.begin()
+s = box.schema.space.create('ddl_test', {engine = engine, id = id})
+box.commit();
+---
+...
+box.begin()
+s:create_index('pk')
+box.rollback();
+---
+...
+box.begin()
+s:create_index('pk')
+box.commit();
+---
+...
+s:replace({1});
+---
+- [1]
+...
+s:replace({2});
+---
+- [2]
+...
+s:replace({3});
+---
+- [3]
+...
+box.begin()
+s:truncate()
+box.commit();
+---
+...
+s:select();
+---
+- []
+...
+box.begin()
+box.schema.user.grant('guest', 'write', 'space', 'ddl_test')
+box.rollback();
+---
+...
+box.begin()
+box.schema.user.grant('guest', 'write', 'space', 'ddl_test')
+box.commit();
+---
+...
+box.begin()
+box.schema.user.revoke('guest', 'write', 'space', 'ddl_test')
+box.rollback();
+---
+...
+box.begin()
+box.schema.user.revoke('guest', 'write', 'space', 'ddl_test')
+box.commit();
+---
+...
+-- index and space drop are not currently supported (because of truncate)
+s.index.pk:drop();
+---
+...
+s:drop();
+---
+...
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index cdaf7a5bf..636f6c3b9 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -759,6 +759,9 @@ i3:select()
 
 -- Check that recovery works.
 inspector:cmd("restart server default")
+test_run = require('test_run')
+inspector = test_run.new()
+engine = inspector:get_cfg('engine')
 
 s = box.space.test
 s.index.i1:select()
@@ -775,3 +778,55 @@ s.index.i1:select()
 box.snapshot()
 
 s:drop()
+
+-- test ddl operation within begin/commit/rollback
+-- acquire free space id
+space = box.schema.space.create('ddl_test', {engine = engine})
+id = space.id
+space:drop()
+
+inspector:cmd("setopt delimiter ';'")
+box.begin()
+s = box.schema.space.create('ddl_test', {engine = engine, id = id})
+box.rollback();
+
+box.begin()
+s = box.schema.space.create('ddl_test', {engine = engine, id = id})
+box.commit();
+
+box.begin()
+s:create_index('pk')
+box.rollback();
+
+box.begin()
+s:create_index('pk')
+box.commit();
+
+s:replace({1});
+s:replace({2});
+s:replace({3});
+
+box.begin()
+s:truncate()
+box.commit();
+s:select();
+
+box.begin()
+box.schema.user.grant('guest', 'write', 'space', 'ddl_test')
+box.rollback();
+
+box.begin()
+box.schema.user.grant('guest', 'write', 'space', 'ddl_test')
+box.commit();
+
+box.begin()
+box.schema.user.revoke('guest', 'write', 'space', 'ddl_test')
+box.rollback();
+
+box.begin()
+box.schema.user.revoke('guest', 'write', 'space', 'ddl_test')
+box.commit();
+
+-- index and space drop are not currently supported (because of truncate)
+s.index.pk:drop();
+s:drop();
diff --git a/test/engine/truncate.result b/test/engine/truncate.result
index b4de787fb..277e7bda6 100644
--- a/test/engine/truncate.result
+++ b/test/engine/truncate.result
@@ -24,14 +24,13 @@ box.begin()
 ...
 s:truncate()
 ---
-- error: DDL does not support multi-statement transactions
 ...
 box.commit()
 ---
 ...
 s:select()
 ---
-- - [123]
+- []
 ...
 s:drop()
 ---
diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
index 2984d4c21..924e57b58 100755
--- a/test/sql-tap/trigger1.test.lua
+++ b/test/sql-tap/trigger1.test.lua
@@ -849,28 +849,24 @@ test:do_catchsql_test(
         -- </trigger1-16.7>
     })
 
-test:do_catchsql_test(
+test:do_execsql_test(
     "trigger1-16.8",
     [[
         START TRANSACTION;
           CREATE TRIGGER tr168 INSERT ON tA BEGIN
             INSERT INTO t16 values(1);
           END;
+        ROLLBACK;
    ]], {
-        1, [[Space _trigger does not support multi-statement transactions]]
 })
 
-test:execsql [[
-    ROLLBACK;
-]]
-
-test:do_catchsql_test(
+test:do_execsql_test(
     "trigger1-16.9",
     [[
         START TRANSACTION;
           DROP TRIGGER t16err3;
+        ROLLBACK;
    ]], {
-        1, [[Space _trigger does not support multi-statement transactions]]
 })
 -- MUST_WORK_TEST
 -- #-------------------------------------------------------------------------
diff --git a/test/sql/delete.result b/test/sql/delete.result
index e024dd697..dcefa8d5f 100644
--- a/test/sql/delete.result
+++ b/test/sql/delete.result
@@ -76,17 +76,21 @@ box.sql.execute("INSERT INTO t1 VALUES(1, 1, 'one');")
 box.sql.execute("INSERT INTO t1 VALUES(2, 2, 'two');")
 ---
 ...
--- Can't truncate in transaction.
+-- Truncate rollback
 box.sql.execute("START TRANSACTION")
 ---
 ...
 box.sql.execute("TRUNCATE TABLE t1;")
 ---
-- error: DDL does not support multi-statement transactions
 ...
 box.sql.execute("ROLLBACK")
 ---
 ...
+box.sql.execute("SELECT * FROM t1")
+---
+- - [1, 1, 'one']
+  - [2, 2, 'two']
+...
 -- Can't truncate view.
 box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;")
 ---
diff --git a/test/sql/delete.test.lua b/test/sql/delete.test.lua
index 5a0813071..b61a993a8 100644
--- a/test/sql/delete.test.lua
+++ b/test/sql/delete.test.lua
@@ -50,10 +50,11 @@ box.sql.execute("CREATE TABLE t1(id INT PRIMARY KEY, a INT, b TEXT);")
 box.sql.execute("INSERT INTO t1 VALUES(1, 1, 'one');")
 box.sql.execute("INSERT INTO t1 VALUES(2, 2, 'two');")
 
--- Can't truncate in transaction.
+-- Truncate rollback
 box.sql.execute("START TRANSACTION")
 box.sql.execute("TRUNCATE TABLE t1;")
 box.sql.execute("ROLLBACK")
+box.sql.execute("SELECT * FROM t1")
 
 -- Can't truncate view.
 box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;")
-- 
2.21.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback
  2019-03-20 20:31 ` [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback Georgy Kirichenko
@ 2019-03-21 20:16   ` Vladimir Davydov
  0 siblings, 0 replies; 5+ messages in thread
From: Vladimir Davydov @ 2019-03-21 20:16 UTC (permalink / raw)
  To: Georgy Kirichenko; +Cc: tarantool-patches

On Wed, Mar 20, 2019 at 11:31:33PM +0300, Georgy Kirichenko wrote:
> Abort index create if truncate couldn't be finished because of rollback
> or error. Without this vinyl will fail because of internal scheduler
> assertion.
> 
> Needed for: 2798
> ---
>  src/box/alter.cc            |  8 ++++++++
>  test/engine/errinj.result   | 34 ++++++++++++++++++++++++++++++++++
>  test/engine/errinj.test.lua | 12 ++++++++++++
>  test/engine/suite.ini       |  1 +
>  4 files changed, 55 insertions(+)
>  create mode 100644 test/engine/errinj.result
>  create mode 100644 test/engine/errinj.test.lua
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 431da12da..84e74ee89 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -1312,6 +1312,7 @@ public:
>  	uint32_t iid;
>  	virtual void prepare(struct alter_space *alter);
>  	virtual void commit(struct alter_space *alter, int64_t signature);
> +	virtual void rollback(struct alter_space *alter);
>  };
>  
>  void
> @@ -1349,6 +1350,13 @@ TruncateIndex::commit(struct alter_space *alter, int64_t signature)
>  	index_commit_create(new_index, signature);
>  }
>  
> +void
> +TruncateIndex::rollback(struct alter_space *alter)
> +{
> +	struct index *new_index = space_index(alter->new_space, iid);
> +	index_abort_create(new_index);
> +}
> +

Strictly speaking, it isn't quite correct to do this from 'rollback'
callback, because the latter is called only if alter_space_do()
succeeded, which may not be the case. I think you better do this from
TruncateIndex destructor. Take a look at CreateIndex and RebuildIndex
for the reference.

>  /**
>   * UpdateSchemaVersion - increment schema_version. Used on
>   * in alter_space_do(), i.e. when creating or dropping
> diff --git a/test/engine/errinj.result b/test/engine/errinj.result
> new file mode 100644
> index 000000000..37fc280b9
> --- /dev/null
> +++ b/test/engine/errinj.result
> @@ -0,0 +1,34 @@
> +test_run = require('test_run')
> +---
> +...
> +inspector = test_run.new()
> +---
> +...
> +engine = inspector:get_cfg('engine')
> +---
> +...
> +errinj = box.error.injection
> +---
> +...
> +-- test truncate rollback does not abort

I can't parse this comment. Let's rephrase it:

 --
 -- Check that on WAL write error space.truncate is rolled back properly.
 --

> +s = box.schema.space.create('truncate_rollback', {engine = engine})
> +---
> +...
> +_ = s:create_index('pk')

Please create a secondary index too.

Also, insert a few rows into the index and check that they are still
there after failed s.truncate.

> +---
> +...
> +errinj.set('ERRINJ_WAL_IO', true)
> +---
> +- ok
> +...
> +s:truncate()
> +---
> +- error: Failed to write to disk
> +...
> +errinj.set('ERRINJ_WAL_IO', false)
> +---
> +- ok
> +...
> +s:drop()
> +---
> +...

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-03-21 20:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 20:31 [tarantool-patches] [PATCH 0/3] Enable ddl between begin and commit statements Georgy Kirichenko
2019-03-20 20:31 ` [tarantool-patches] [PATCH 1/3] Abort vinyl index creation in case of truncation rollback Georgy Kirichenko
2019-03-21 20:16   ` Vladimir Davydov
2019-03-20 20:31 ` [tarantool-patches] [PATCH 2/3] Synchronize lua schema update with space cache Georgy Kirichenko
2019-03-20 20:31 ` [tarantool-patches] [PATCH 3/3] Require for single statement not autocommit in case of ddl Georgy Kirichenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox