[PATCH 0/6] Introduce blackhole engine

[PATCH 0/6] Introduce blackhole engine

[Vladimir Davydov]

In the scope of #2129, we need to store deferred DELETE statements
generated during primary index compaction in the WAL so that we can
recover them after restart. This patch set introduces a new engine for
this purpose - blackhole. Spaces that use this engine may be written to,
but not read from (they don't even support creation of indexes). All
operations done on a blackhole space are logged in the WAL and hence
replayed during recovery, which is just what we need.

https://github.com/tarantool/tarantool/issues/2129
https://github.com/tarantool/tarantool/commits/dv/blackhole

Nikita Pettik (1):
  Rework memtx replace function

Vladimir Davydov (5):
  Add generic engine, space, index method stubs
  Merge sysview_index.[hc] and sysview_engine.[hc]
  txn: unify txn_stmt tuples reference counting rules
  space: call before_replace trigger even if space has no indexes
  Introduce blackhole engine

 src/box/CMakeLists.txt                   |   4 +-
 src/box/blackhole.c                      | 210 ++++++++++++++++
 src/box/{sysview_index.h => blackhole.h} |  40 ++-
 src/box/box.cc                           |   6 +-
 src/box/engine.c                         | 161 ++++++++++++
 src/box/engine.h                         |  28 +++
 src/box/index.cc                         |  13 +
 src/box/index.h                          |   3 +
 src/box/memtx_engine.c                   |  47 +---
 src/box/memtx_space.c                    | 100 +++++---
 src/box/memtx_space.h                    |  20 +-
 src/box/space.c                          | 109 +++++++--
 src/box/space.h                          |  14 ++
 src/box/{sysview_index.c => sysview.c}   | 195 +++++++++++++--
 src/box/{sysview_engine.h => sysview.h}  |  16 +-
 src/box/sysview_engine.c                 | 406 -------------------------------
 src/box/txn.c                            |  19 ++
 src/box/vinyl.c                          |  42 +---
 test/box/blackhole.result                | 177 ++++++++++++++
 test/box/blackhole.test.lua              |  64 +++++
 20 files changed, 1068 insertions(+), 606 deletions(-)
 create mode 100644 src/box/blackhole.c
 rename src/box/{sysview_index.h => blackhole.h} (69%)
 rename src/box/{sysview_index.c => sysview.c} (70%)
 rename src/box/{sysview_engine.h => sysview.h} (84%)
 delete mode 100644 src/box/sysview_engine.c
 create mode 100644 test/box/blackhole.result
 create mode 100644 test/box/blackhole.test.lua

-- 
2.11.0

[PATCH 1/6] Add generic engine, space, index method stubs

[Vladimir Davydov]

This should reduce maintenance burden and help us introduce a new
engine.
---
 src/box/engine.c         | 161 ++++++++++++++++++++++++++
 src/box/engine.h         |  28 +++++
 src/box/index.cc         |  13 +++
 src/box/index.h          |   3 +
 src/box/memtx_engine.c   |  17 +--
 src/box/space.c          |  73 ++++++++++++
 src/box/space.h          |  14 +++
 src/box/sysview_engine.c | 287 +++++------------------------------------------
 src/box/sysview_index.c  |  20 +---
 src/box/vinyl.c          |  17 +--
 10 files changed, 327 insertions(+), 306 deletions(-)

diff --git a/src/box/engine.c b/src/box/engine.c
index 82293fd1..2a30dcdd 100644
--- a/src/box/engine.c
+++ b/src/box/engine.c
@@ -196,3 +196,164 @@ engine_reset_stat(void)
 	engine_foreach(engine)
 		engine->vtab->reset_stat(engine);
 }
+
+/* {{{ Virtual method stubs */
+
+int
+generic_engine_join(struct engine *engine, const struct vclock *vclock,
+		    struct xstream *stream)
+{
+	(void)engine;
+	(void)vclock;
+	(void)stream;
+	return 0;
+}
+
+int
+generic_engine_begin(struct engine *engine, struct txn *txn)
+{
+	(void)engine;
+	(void)txn;
+	return 0;
+}
+
+int
+generic_engine_begin_statement(struct engine *engine, struct txn *txn)
+{
+	(void)engine;
+	(void)txn;
+	return 0;
+}
+
+int
+generic_engine_prepare(struct engine *engine, struct txn *txn)
+{
+	(void)engine;
+	(void)txn;
+	return 0;
+}
+
+void
+generic_engine_commit(struct engine *engine, struct txn *txn)
+{
+	(void)engine;
+	(void)txn;
+}
+
+void
+generic_engine_rollback_statement(struct engine *engine, struct txn *txn,
+				  struct txn_stmt *stmt)
+{
+	(void)engine;
+	(void)txn;
+	(void)stmt;
+}
+
+void
+generic_engine_rollback(struct engine *engine, struct txn *txn)
+{
+	(void)engine;
+	(void)txn;
+}
+
+int
+generic_engine_bootstrap(struct engine *engine)
+{
+	(void)engine;
+	return 0;
+}
+
+int
+generic_engine_begin_initial_recovery(struct engine *engine,
+				      const struct vclock *vclock)
+{
+	(void)engine;
+	(void)vclock;
+	return 0;
+}
+
+int
+generic_engine_begin_final_recovery(struct engine *engine)
+{
+	(void)engine;
+	return 0;
+}
+
+int
+generic_engine_end_recovery(struct engine *engine)
+{
+	(void)engine;
+	return 0;
+}
+
+int
+generic_engine_begin_checkpoint(struct engine *engine)
+{
+	(void)engine;
+	return 0;
+}
+
+int
+generic_engine_wait_checkpoint(struct engine *engine,
+			       const struct vclock *vclock)
+{
+	(void)engine;
+	(void)vclock;
+	return 0;
+}
+
+void
+generic_engine_commit_checkpoint(struct engine *engine,
+				 const struct vclock *vclock)
+{
+	(void)engine;
+	(void)vclock;
+}
+
+void
+generic_engine_abort_checkpoint(struct engine *engine)
+{
+	(void)engine;
+}
+
+int
+generic_engine_collect_garbage(struct engine *engine, int64_t lsn)
+{
+	(void)engine;
+	(void)lsn;
+	return 0;
+}
+
+int
+generic_engine_backup(struct engine *engine, const struct vclock *vclock,
+		      engine_backup_cb cb, void *cb_arg)
+{
+	(void)engine;
+	(void)vclock;
+	(void)cb;
+	(void)cb_arg;
+	return 0;
+}
+
+void
+generic_engine_memory_stat(struct engine *engine,
+			   struct engine_memory_stat *stat)
+{
+	(void)engine;
+	(void)stat;
+}
+
+void
+generic_engine_reset_stat(struct engine *engine)
+{
+	(void)engine;
+}
+
+int
+generic_engine_check_space_def(struct space_def *def)
+{
+	(void)def;
+	return 0;
+}
+
+/* }}} */
diff --git a/src/box/engine.h b/src/box/engine.h
index 825f059e..e10eaec4 100644
--- a/src/box/engine.h
+++ b/src/box/engine.h
@@ -336,6 +336,34 @@ engine_memory_stat(struct engine_memory_stat *stat);
 void
 engine_reset_stat(void);
 
+/*
+ * Virtual method stubs.
+ */
+int generic_engine_join(struct engine *, const struct vclock *,
+			struct xstream *);
+int generic_engine_begin(struct engine *, struct txn *);
+int generic_engine_begin_statement(struct engine *, struct txn *);
+int generic_engine_prepare(struct engine *, struct txn *);
+void generic_engine_commit(struct engine *, struct txn *);
+void generic_engine_rollback_statement(struct engine *, struct txn *,
+				       struct txn_stmt *);
+void generic_engine_rollback(struct engine *, struct txn *);
+int generic_engine_bootstrap(struct engine *);
+int generic_engine_begin_initial_recovery(struct engine *,
+					  const struct vclock *);
+int generic_engine_begin_final_recovery(struct engine *);
+int generic_engine_end_recovery(struct engine *);
+int generic_engine_begin_checkpoint(struct engine *);
+int generic_engine_wait_checkpoint(struct engine *, const struct vclock *);
+void generic_engine_commit_checkpoint(struct engine *, const struct vclock *);
+void generic_engine_abort_checkpoint(struct engine *);
+int generic_engine_collect_garbage(struct engine *, int64_t);
+int generic_engine_backup(struct engine *, const struct vclock *,
+			  engine_backup_cb, void *);
+void generic_engine_memory_stat(struct engine *, struct engine_memory_stat *);
+void generic_engine_reset_stat(struct engine *);
+int generic_engine_check_space_def(struct space_def *);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
diff --git a/src/box/index.cc b/src/box/index.cc
index 1e3f7afb..ea24b3aa 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -582,6 +582,13 @@ bool generic_index_depends_on_pk(struct index *)
 	return false;
 }
 
+bool
+generic_index_def_change_requires_rebuild(struct index *,
+					  const struct index_def *)
+{
+	return true;
+}
+
 ssize_t
 generic_index_size(struct index *index)
 {
@@ -589,6 +596,12 @@ generic_index_size(struct index *index)
 	return -1;
 }
 
+ssize_t
+generic_index_bsize(struct index *)
+{
+	return 0;
+}
+
 int
 generic_index_min(struct index *index, const char *key,
 		  uint32_t part_count, struct tuple **result)
diff --git a/src/box/index.h b/src/box/index.h
index 686e7a1a..0a1ac611 100644
--- a/src/box/index.h
+++ b/src/box/index.h
@@ -667,6 +667,9 @@ void generic_index_commit_modify(struct index *, int64_t);
 void generic_index_commit_drop(struct index *, int64_t);
 void generic_index_update_def(struct index *);
 bool generic_index_depends_on_pk(struct index *);
+bool generic_index_def_change_requires_rebuild(struct index *,
+					       const struct index_def *);
+ssize_t generic_index_bsize(struct index *);
 ssize_t generic_index_size(struct index *);
 int generic_index_min(struct index *, const char *, uint32_t, struct tuple **);
 int generic_index_max(struct index *, const char *, uint32_t, struct tuple **);
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index c210afbe..a1ce4fff 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -966,19 +966,6 @@ memtx_engine_memory_stat(struct engine *engine, struct engine_memory_stat *stat)
 	stat->index += index_stats.totals.used;
 }
 
-static void
-memtx_engine_reset_stat(struct engine *engine)
-{
-	(void)engine;
-}
-
-static int
-memtx_engine_check_space_def(struct space_def *def)
-{
-	(void)def;
-	return 0;
-}
-
 static const struct engine_vtab memtx_engine_vtab = {
 	/* .shutdown = */ memtx_engine_shutdown,
 	/* .create_space = */ memtx_engine_create_space,
@@ -1000,8 +987,8 @@ static const struct engine_vtab memtx_engine_vtab = {
 	/* .collect_garbage = */ memtx_engine_collect_garbage,
 	/* .backup = */ memtx_engine_backup,
 	/* .memory_stat = */ memtx_engine_memory_stat,
-	/* .reset_stat = */ memtx_engine_reset_stat,
-	/* .check_space_def = */ memtx_engine_check_space_def,
+	/* .reset_stat = */ generic_engine_reset_stat,
+	/* .check_space_def = */ generic_engine_check_space_def,
 };
 
 /**
diff --git a/src/box/space.c b/src/box/space.c
index a325a42e..fc59dbe3 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -505,3 +505,76 @@ space_execute_dml(struct space *space, struct txn *txn,
 	}
 	return 0;
 }
+
+/* {{{ Virtual method stubs */
+
+size_t
+generic_space_bsize(struct space *space)
+{
+	(void)space;
+	return 0;
+}
+
+int
+generic_space_apply_initial_join_row(struct space *space,
+				     struct request *request)
+{
+	(void)space;
+	(void)request;
+	return 0;
+}
+
+void
+generic_init_system_space(struct space *space)
+{
+	(void)space;
+}
+
+int
+generic_space_check_index_def(struct space *space, struct index_def *index_def)
+{
+	(void)space;
+	(void)index_def;
+	return 0;
+}
+
+int
+generic_space_add_primary_key(struct space *space)
+{
+	(void)space;
+	return 0;
+}
+
+void
+generic_space_drop_primary_key(struct space *space)
+{
+	(void)space;
+}
+
+int
+generic_space_check_format(struct space *space, struct tuple_format *format)
+{
+	(void)space;
+	(void)format;
+	return 0;
+}
+
+int
+generic_space_build_index(struct space *src_space, struct index *new_index,
+			  struct tuple_format *new_format)
+{
+	(void)src_space;
+	(void)new_index;
+	(void)new_format;
+	return 0;
+}
+
+int
+generic_space_prepare_alter(struct space *old_space, struct space *new_space)
+{
+	(void)old_space;
+	(void)new_space;
+	return 0;
+}
+
+/* }}} */
diff --git a/src/box/space.h b/src/box/space.h
index ae32e6df..8888ec80 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -398,6 +398,20 @@ space_dump_def(const struct space *space, struct rlist *key_list);
 void
 space_fill_index_map(struct space *space);
 
+/*
+ * Virtual method stubs.
+ */
+size_t generic_space_bsize(struct space *);
+int generic_space_apply_initial_join_row(struct space *, struct request *);
+void generic_init_system_space(struct space *);
+int generic_space_check_index_def(struct space *, struct index_def *);
+int generic_space_add_primary_key(struct space *space);
+void generic_space_drop_primary_key(struct space *space);
+int generic_space_check_format(struct space *, struct tuple_format *);
+int generic_space_build_index(struct space *, struct index *,
+			      struct tuple_format *);
+int generic_space_prepare_alter(struct space *, struct space *);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
index 762bf93e..f2f27381 100644
--- a/src/box/sysview_engine.c
+++ b/src/box/sysview_engine.c
@@ -39,23 +39,6 @@ sysview_space_destroy(struct space *space)
 	free(space);
 }
 
-static size_t
-sysview_space_bsize(struct space *space)
-{
-	(void)space;
-	return 0;
-}
-
-static int
-sysview_space_apply_initial_join_row(struct space *space,
-				     struct request *request)
-{
-	(void)space;
-	(void)request;
-	unreachable();
-	return 0;
-}
-
 static int
 sysview_space_execute_replace(struct space *space, struct txn *txn,
 			      struct request *request, struct tuple **result)
@@ -99,21 +82,6 @@ sysview_space_execute_upsert(struct space *space, struct txn *txn,
 	return -1;
 }
 
-static void
-sysview_init_system_space(struct space *space)
-{
-	(void)space;
-	unreachable();
-}
-
-static int
-sysview_space_check_index_def(struct space *space, struct index_def *index_def)
-{
-	(void)space;
-	(void)index_def;
-	return 0;
-}
-
 static struct index *
 sysview_space_create_index(struct space *space, struct index_def *index_def)
 {
@@ -122,63 +90,23 @@ sysview_space_create_index(struct space *space, struct index_def *index_def)
 						 space_name(space));
 }
 
-static int
-sysview_space_add_primary_key(struct space *space)
-{
-	(void)space;
-	return 0;
-}
-
-static void
-sysview_space_drop_primary_key(struct space *space)
-{
-	(void)space;
-}
-
-static int
-sysview_space_build_index(struct space *src_space, struct index *new_index,
-			  struct tuple_format *new_format)
-{
-	(void)src_space;
-	(void)new_index;
-	(void)new_format;
-	return 0;
-}
-
-static int
-sysview_space_prepare_alter(struct space *old_space, struct space *new_space)
-{
-	(void)old_space;
-	(void)new_space;
-	return 0;
-}
-
-static int
-sysview_space_check_format(struct space *space, struct tuple_format *format)
-{
-	(void)space;
-	(void)format;
-	unreachable();
-	return 0;
-}
-
 static const struct space_vtab sysview_space_vtab = {
 	/* .destroy = */ sysview_space_destroy,
-	/* .bsize = */ sysview_space_bsize,
-	/* .apply_initial_join_row = */ sysview_space_apply_initial_join_row,
+	/* .bsize = */ generic_space_bsize,
+	/* .apply_initial_join_row = */ generic_space_apply_initial_join_row,
 	/* .execute_replace = */ sysview_space_execute_replace,
 	/* .execute_delete = */ sysview_space_execute_delete,
 	/* .execute_update = */ sysview_space_execute_update,
 	/* .execute_upsert = */ sysview_space_execute_upsert,
-	/* .init_system_space = */ sysview_init_system_space,
-	/* .check_index_def = */ sysview_space_check_index_def,
+	/* .init_system_space = */ generic_init_system_space,
+	/* .check_index_def = */ generic_space_check_index_def,
 	/* .create_index = */ sysview_space_create_index,
-	/* .add_primary_key = */ sysview_space_add_primary_key,
-	/* .drop_primary_key = */ sysview_space_drop_primary_key,
-	/* .check_format = */ sysview_space_check_format,
-	/* .build_index = */ sysview_space_build_index,
+	/* .add_primary_key = */ generic_space_add_primary_key,
+	/* .drop_primary_key = */ generic_space_drop_primary_key,
+	/* .check_format = */ generic_space_check_format,
+	/* .build_index = */ generic_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
-	/* .prepare_alter = */ sysview_space_prepare_alter,
+	/* .prepare_alter = */ generic_space_prepare_alter,
 };
 
 static void
@@ -208,186 +136,29 @@ sysview_engine_create_space(struct engine *engine, struct space_def *def,
 	return space;
 }
 
-static int
-sysview_engine_begin(struct engine *engine, struct txn *txn)
-{
-	(void)engine;
-	(void)txn;
-	return 0;
-}
-
-static int
-sysview_engine_begin_statement(struct engine *engine, struct txn *txn)
-{
-	(void)engine;
-	(void)txn;
-	return 0;
-}
-
-static int
-sysview_engine_prepare(struct engine *engine, struct txn *txn)
-{
-	(void)engine;
-	(void)txn;
-	return 0;
-}
-
-static void
-sysview_engine_commit(struct engine *engine, struct txn *txn)
-{
-	(void)engine;
-	(void)txn;
-}
-
-static void
-sysview_engine_rollback(struct engine *engine, struct txn *txn)
-{
-	(void)engine;
-	(void)txn;
-}
-
-static void
-sysview_engine_rollback_statement(struct engine *engine, struct txn *txn,
-				  struct txn_stmt *stmt)
-{
-	(void)engine;
-	(void)txn;
-	(void)stmt;
-}
-
-static int
-sysview_engine_bootstrap(struct engine *engine)
-{
-	(void)engine;
-	return 0;
-}
-
-static int
-sysview_engine_begin_initial_recovery(struct engine *engine,
-				      const struct vclock *vclock)
-{
-	(void)engine;
-	(void)vclock;
-	return 0;
-}
-
-static int
-sysview_engine_begin_final_recovery(struct engine *engine)
-{
-	(void)engine;
-	return 0;
-}
-
-static int
-sysview_engine_end_recovery(struct engine *engine)
-{
-	(void)engine;
-	return 0;
-}
-
-static int
-sysview_engine_join(struct engine *engine, const struct vclock *vclock,
-		    struct xstream *stream)
-{
-	(void)engine;
-	(void)vclock;
-	(void)stream;
-	return 0;
-}
-
-static int
-sysview_engine_begin_checkpoint(struct engine *engine)
-{
-	(void)engine;
-	return 0;
-}
-
-static int
-sysview_engine_wait_checkpoint(struct engine *engine,
-			       const struct vclock *vclock)
-{
-	(void)engine;
-	(void)vclock;
-	return 0;
-}
-
-static void
-sysview_engine_commit_checkpoint(struct engine *engine,
-				 const struct vclock *vclock)
-{
-	(void)engine;
-	(void)vclock;
-}
-
-static void
-sysview_engine_abort_checkpoint(struct engine *engine)
-{
-	(void)engine;
-}
-
-static int
-sysview_engine_collect_garbage(struct engine *engine, int64_t lsn)
-{
-	(void)engine;
-	(void)lsn;
-	return 0;
-}
-
-static int
-sysview_engine_backup(struct engine *engine, const struct vclock *vclock,
-		      engine_backup_cb cb, void *cb_arg)
-{
-	(void)engine;
-	(void)vclock;
-	(void)cb;
-	(void)cb_arg;
-	return 0;
-}
-
-static void
-sysview_engine_memory_stat(struct engine *engine,
-			   struct engine_memory_stat *stat)
-{
-	(void)engine;
-	(void)stat;
-}
-
-static void
-sysview_engine_reset_stat(struct engine *engine)
-{
-	(void)engine;
-}
-
-static int
-sysview_engine_check_space_def(struct space_def *def)
-{
-	(void)def;
-	return 0;
-}
-
 static const struct engine_vtab sysview_engine_vtab = {
 	/* .shutdown = */ sysview_engine_shutdown,
 	/* .create_space = */ sysview_engine_create_space,
-	/* .join = */ sysview_engine_join,
-	/* .begin = */ sysview_engine_begin,
-	/* .begin_statement = */ sysview_engine_begin_statement,
-	/* .prepare = */ sysview_engine_prepare,
-	/* .commit = */ sysview_engine_commit,
-	/* .rollback_statement = */ sysview_engine_rollback_statement,
-	/* .rollback = */ sysview_engine_rollback,
-	/* .bootstrap = */ sysview_engine_bootstrap,
-	/* .begin_initial_recovery = */ sysview_engine_begin_initial_recovery,
-	/* .begin_final_recovery = */ sysview_engine_begin_final_recovery,
-	/* .end_recovery = */ sysview_engine_end_recovery,
-	/* .begin_checkpoint = */ sysview_engine_begin_checkpoint,
-	/* .wait_checkpoint = */ sysview_engine_wait_checkpoint,
-	/* .commit_checkpoint = */ sysview_engine_commit_checkpoint,
-	/* .abort_checkpoint = */ sysview_engine_abort_checkpoint,
-	/* .collect_garbage = */ sysview_engine_collect_garbage,
-	/* .backup = */ sysview_engine_backup,
-	/* .memory_stat = */ sysview_engine_memory_stat,
-	/* .reset_stat = */ sysview_engine_reset_stat,
-	/* .check_space_def = */ sysview_engine_check_space_def,
+	/* .join = */ generic_engine_join,
+	/* .begin = */ generic_engine_begin,
+	/* .begin_statement = */ generic_engine_begin_statement,
+	/* .prepare = */ generic_engine_prepare,
+	/* .commit = */ generic_engine_commit,
+	/* .rollback_statement = */ generic_engine_rollback_statement,
+	/* .rollback = */ generic_engine_rollback,
+	/* .bootstrap = */ generic_engine_bootstrap,
+	/* .begin_initial_recovery = */ generic_engine_begin_initial_recovery,
+	/* .begin_final_recovery = */ generic_engine_begin_final_recovery,
+	/* .end_recovery = */ generic_engine_end_recovery,
+	/* .begin_checkpoint = */ generic_engine_begin_checkpoint,
+	/* .wait_checkpoint = */ generic_engine_wait_checkpoint,
+	/* .commit_checkpoint = */ generic_engine_commit_checkpoint,
+	/* .abort_checkpoint = */ generic_engine_abort_checkpoint,
+	/* .collect_garbage = */ generic_engine_collect_garbage,
+	/* .backup = */ generic_engine_backup,
+	/* .memory_stat = */ generic_engine_memory_stat,
+	/* .reset_stat = */ generic_engine_reset_stat,
+	/* .check_space_def = */ generic_engine_check_space_def,
 };
 
 struct sysview_engine *
diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c
index 5e3d8423..309618ca 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview_index.c
@@ -84,22 +84,6 @@ sysview_index_destroy(struct index *index)
 	free(index);
 }
 
-static ssize_t
-sysview_index_bsize(struct index *index)
-{
-	(void)index;
-	return 0;
-}
-
-static bool
-sysview_index_def_change_requires_rebuild(struct index *index,
-					  const struct index_def *new_def)
-{
-	(void)index;
-	(void)new_def;
-	return true;
-}
-
 static struct iterator *
 sysview_index_create_iterator(struct index *base, enum iterator_type type,
 			      const char *key, uint32_t part_count)
@@ -177,9 +161,9 @@ static const struct index_vtab sysview_index_vtab = {
 	/* .update_def = */ generic_index_update_def,
 	/* .depends_on_pk = */ generic_index_depends_on_pk,
 	/* .def_change_requires_rebuild = */
-		sysview_index_def_change_requires_rebuild,
+		generic_index_def_change_requires_rebuild,
 	/* .size = */ generic_index_size,
-	/* .bsize = */ sysview_index_bsize,
+	/* .bsize = */ generic_index_bsize,
 	/* .min = */ generic_index_min,
 	/* .max = */ generic_index_max,
 	/* .random = */ generic_index_random,
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index a802b169..f02fa638 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1002,13 +1002,6 @@ vinyl_index_def_change_requires_rebuild(struct index *index,
 	return false;
 }
 
-static void
-vinyl_init_system_space(struct space *space)
-{
-	(void)space;
-	unreachable();
-}
-
 static int
 vinyl_space_prepare_alter(struct space *old_space, struct space *new_space)
 {
@@ -1159,12 +1152,6 @@ vinyl_space_add_primary_key(struct space *space)
 	return vinyl_index_open(space->index[0]);
 }
 
-static void
-vinyl_space_drop_primary_key(struct space *space)
-{
-	(void)space;
-}
-
 static size_t
 vinyl_space_bsize(struct space *space)
 {
@@ -4488,11 +4475,11 @@ static const struct space_vtab vinyl_space_vtab = {
 	/* .execute_delete = */ vinyl_space_execute_delete,
 	/* .execute_update = */ vinyl_space_execute_update,
 	/* .execute_upsert = */ vinyl_space_execute_upsert,
-	/* .init_system_space = */ vinyl_init_system_space,
+	/* .init_system_space = */ generic_init_system_space,
 	/* .check_index_def = */ vinyl_space_check_index_def,
 	/* .create_index = */ vinyl_space_create_index,
 	/* .add_primary_key = */ vinyl_space_add_primary_key,
-	/* .drop_primary_key = */ vinyl_space_drop_primary_key,
+	/* .drop_primary_key = */ generic_space_drop_primary_key,
 	/* .check_format = */ vinyl_space_check_format,
 	/* .build_index = */ vinyl_space_build_index,
 	/* .swap_index = */ vinyl_space_swap_index,
-- 
2.11.0

[PATCH 2/6] Merge sysview_index.[hc] and sysview_engine.[hc]

[Vladimir Davydov]

They are fairly small and closely related so let's merge them and call
the result sysview.[hc].
---
 src/box/CMakeLists.txt                  |   3 +-
 src/box/box.cc                          |   2 +-
 src/box/{sysview_index.c => sysview.c}  | 175 +++++++++++++++++++++++++++++--
 src/box/{sysview_engine.h => sysview.h} |  16 +--
 src/box/sysview_engine.c                | 177 --------------------------------
 src/box/sysview_index.h                 |  60 -----------
 6 files changed, 174 insertions(+), 259 deletions(-)
 rename src/box/{sysview_index.c => sysview.c} (71%)
 rename src/box/{sysview_engine.h => sysview.h} (84%)
 delete mode 100644 src/box/sysview_engine.c
 delete mode 100644 src/box/sysview_index.h

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index f4899091..fabeb3fd 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -68,8 +68,7 @@ add_library(box STATIC
     engine.c
     memtx_engine.c
     memtx_space.c
-    sysview_engine.c
-    sysview_index.c
+    sysview.c
     vinyl.c
     vy_stmt.c
     vy_mem.c
diff --git a/src/box/box.cc b/src/box/box.cc
index b07eefa8..9c3f87f0 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -50,7 +50,7 @@
 #include "schema.h"
 #include "engine.h"
 #include "memtx_engine.h"
-#include "sysview_engine.h"
+#include "sysview.h"
 #include "vinyl.h"
 #include "space.h"
 #include "index.h"
diff --git a/src/box/sysview_index.c b/src/box/sysview.c
similarity index 71%
rename from src/box/sysview_index.c
rename to src/box/sysview.c
index 309618ca..fb4ecf2d 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview.c
@@ -28,17 +28,41 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-#include "sysview_index.h"
-#include "sysview_engine.h"
+#include "sysview.h"
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdlib.h>
 #include <small/mempool.h>
-#include "fiber.h"
+
+#include "diag.h"
+#include "error.h"
+#include "errcode.h"
 #include "schema.h"
 #include "sequence.h"
 #include "space.h"
+#include "index.h"
+#include "engine.h"
 #include "func.h"
 #include "tuple.h"
 #include "session.h"
 
+typedef bool (*sysview_filter_f)(struct space *, struct tuple *);
+
+struct sysview_engine {
+	struct engine base;
+	/** Memory pool for index iterator. */
+	struct mempool iterator_pool;
+};
+
+struct sysview_index {
+	struct index base;
+	uint32_t source_space_id;
+	uint32_t source_index_id;
+	sysview_filter_f filter;
+};
+
 struct sysview_iterator {
 	struct iterator base;
 	struct iterator *source;
@@ -182,6 +206,55 @@ static const struct index_vtab sysview_index_vtab = {
 	/* .end_build = */ generic_index_end_build,
 };
 
+static void
+sysview_space_destroy(struct space *space)
+{
+	free(space);
+}
+
+static int
+sysview_space_execute_replace(struct space *space, struct txn *txn,
+			      struct request *request, struct tuple **result)
+{
+	(void)txn;
+	(void)request;
+	(void)result;
+	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
+	return -1;
+}
+
+static int
+sysview_space_execute_delete(struct space *space, struct txn *txn,
+			     struct request *request, struct tuple **result)
+{
+	(void)txn;
+	(void)request;
+	(void)result;
+	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
+	return -1;
+}
+
+static int
+sysview_space_execute_update(struct space *space, struct txn *txn,
+			     struct request *request, struct tuple **result)
+{
+	(void)txn;
+	(void)request;
+	(void)result;
+	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
+	return -1;
+}
+
+static int
+sysview_space_execute_upsert(struct space *space, struct txn *txn,
+			     struct request *request)
+{
+	(void)txn;
+	(void)request;
+	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
+	return -1;
+}
+
 /*
  * System view filters.
  * Filter gives access to an object, if one of the following conditions is true:
@@ -329,12 +402,12 @@ vsequence_filter(struct space *source, struct tuple *tuple)
 	       ((PRIV_WRDA | PRIV_X) & effective);
 }
 
-struct sysview_index *
-sysview_index_new(struct sysview_engine *sysview,
-		  struct index_def *def, const char *space_name)
+static struct index *
+sysview_space_create_index(struct space *space, struct index_def *def)
 {
 	assert(def->type == TREE);
 
+	struct sysview_engine *sysview = (struct sysview_engine *)space->engine;
 	if (!mempool_is_initialized(&sysview->iterator_pool)) {
 		mempool_create(&sysview->iterator_pool, cord_slab_cache(),
 			       sizeof(struct sysview_iterator));
@@ -377,7 +450,7 @@ sysview_index_new(struct sysview_engine *sysview,
 		break;
 	default:
 		diag_set(ClientError, ER_MODIFY_INDEX,
-			 def->name, space_name,
+			 def->name, space_name(space),
 			 "unknown space for system view");
 		return NULL;
 	}
@@ -398,5 +471,91 @@ sysview_index_new(struct sysview_engine *sysview,
 	index->source_space_id = source_space_id;
 	index->source_index_id = source_index_id;
 	index->filter = filter;
-	return index;
+	return &index->base;
+}
+
+static const struct space_vtab sysview_space_vtab = {
+	/* .destroy = */ sysview_space_destroy,
+	/* .bsize = */ generic_space_bsize,
+	/* .apply_initial_join_row = */ generic_space_apply_initial_join_row,
+	/* .execute_replace = */ sysview_space_execute_replace,
+	/* .execute_delete = */ sysview_space_execute_delete,
+	/* .execute_update = */ sysview_space_execute_update,
+	/* .execute_upsert = */ sysview_space_execute_upsert,
+	/* .init_system_space = */ generic_init_system_space,
+	/* .check_index_def = */ generic_space_check_index_def,
+	/* .create_index = */ sysview_space_create_index,
+	/* .add_primary_key = */ generic_space_add_primary_key,
+	/* .drop_primary_key = */ generic_space_drop_primary_key,
+	/* .check_format = */ generic_space_check_format,
+	/* .build_index = */ generic_space_build_index,
+	/* .swap_index = */ generic_space_swap_index,
+	/* .prepare_alter = */ generic_space_prepare_alter,
+};
+
+static void
+sysview_engine_shutdown(struct engine *engine)
+{
+	struct sysview_engine *sysview = (struct sysview_engine *)engine;
+	if (mempool_is_initialized(&sysview->iterator_pool))
+		mempool_destroy(&sysview->iterator_pool);
+	free(engine);
+}
+
+static struct space *
+sysview_engine_create_space(struct engine *engine, struct space_def *def,
+			    struct rlist *key_list)
+{
+	struct space *space = (struct space *)calloc(1, sizeof(*space));
+	if (space == NULL) {
+		diag_set(OutOfMemory, sizeof(*space),
+			 "malloc", "struct space");
+		return NULL;
+	}
+	if (space_create(space, engine, &sysview_space_vtab,
+			 def, key_list, NULL) != 0) {
+		free(space);
+		return NULL;
+	}
+	return space;
+}
+
+static const struct engine_vtab sysview_engine_vtab = {
+	/* .shutdown = */ sysview_engine_shutdown,
+	/* .create_space = */ sysview_engine_create_space,
+	/* .join = */ generic_engine_join,
+	/* .begin = */ generic_engine_begin,
+	/* .begin_statement = */ generic_engine_begin_statement,
+	/* .prepare = */ generic_engine_prepare,
+	/* .commit = */ generic_engine_commit,
+	/* .rollback_statement = */ generic_engine_rollback_statement,
+	/* .rollback = */ generic_engine_rollback,
+	/* .bootstrap = */ generic_engine_bootstrap,
+	/* .begin_initial_recovery = */ generic_engine_begin_initial_recovery,
+	/* .begin_final_recovery = */ generic_engine_begin_final_recovery,
+	/* .end_recovery = */ generic_engine_end_recovery,
+	/* .begin_checkpoint = */ generic_engine_begin_checkpoint,
+	/* .wait_checkpoint = */ generic_engine_wait_checkpoint,
+	/* .commit_checkpoint = */ generic_engine_commit_checkpoint,
+	/* .abort_checkpoint = */ generic_engine_abort_checkpoint,
+	/* .collect_garbage = */ generic_engine_collect_garbage,
+	/* .backup = */ generic_engine_backup,
+	/* .memory_stat = */ generic_engine_memory_stat,
+	/* .reset_stat = */ generic_engine_reset_stat,
+	/* .check_space_def = */ generic_engine_check_space_def,
+};
+
+struct sysview_engine *
+sysview_engine_new(void)
+{
+	struct sysview_engine *sysview = calloc(1, sizeof(*sysview));
+	if (sysview == NULL) {
+		diag_set(OutOfMemory, sizeof(*sysview),
+			 "malloc", "struct sysview_engine");
+		return NULL;
+	}
+
+	sysview->base.vtab = &sysview_engine_vtab;
+	sysview->base.name = "sysview";
+	return sysview;
 }
diff --git a/src/box/sysview_engine.h b/src/box/sysview.h
similarity index 84%
rename from src/box/sysview_engine.h
rename to src/box/sysview.h
index fd4ca351..e27ab05f 100644
--- a/src/box/sysview_engine.h
+++ b/src/box/sysview.h
@@ -1,5 +1,5 @@
-#ifndef TARANTOOL_BOX_SYSVIEW_ENGINE_H_INCLUDED
-#define TARANTOOL_BOX_SYSVIEW_ENGINE_H_INCLUDED
+#ifndef TARANTOOL_BOX_SYSVIEW_H_INCLUDED
+#define TARANTOOL_BOX_SYSVIEW_H_INCLUDED
 /*
  * Copyright 2010-2016, Tarantool AUTHORS, please see AUTHORS file.
  *
@@ -30,19 +30,13 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-#include <small/mempool.h>
-
-#include "engine.h"
+#include <stddef.h>
 
 #if defined(__cplusplus)
 extern "C" {
 #endif /* defined(__cplusplus) */
 
-struct sysview_engine {
-	struct engine base;
-	/** Memory pool for index iterator. */
-	struct mempool iterator_pool;
-};
+struct sysview_engine;
 
 struct sysview_engine *
 sysview_engine_new(void);
@@ -63,4 +57,4 @@ sysview_engine_new_xc(void)
 
 #endif /* defined(__plusplus) */
 
-#endif /* TARANTOOL_BOX_SYSVIEW_ENGINE_H_INCLUDED */
+#endif /* TARANTOOL_BOX_SYSVIEW_H_INCLUDED */
diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
deleted file mode 100644
index f2f27381..00000000
--- a/src/box/sysview_engine.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/*
- * Copyright 2010-2016, Tarantool AUTHORS, please see AUTHORS file.
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * 1. Redistributions of source code must retain the above
- *    copyright notice, this list of conditions and the
- *    following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above
- *    copyright notice, this list of conditions and the following
- *    disclaimer in the documentation and/or other materials
- *    provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
- * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
- * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
- * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
- * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-#include "sysview_engine.h"
-#include "sysview_index.h"
-#include "schema.h"
-#include "space.h"
-
-static void
-sysview_space_destroy(struct space *space)
-{
-	free(space);
-}
-
-static int
-sysview_space_execute_replace(struct space *space, struct txn *txn,
-			      struct request *request, struct tuple **result)
-{
-	(void)txn;
-	(void)request;
-	(void)result;
-	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
-	return -1;
-}
-
-static int
-sysview_space_execute_delete(struct space *space, struct txn *txn,
-			     struct request *request, struct tuple **result)
-{
-	(void)txn;
-	(void)request;
-	(void)result;
-	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
-	return -1;
-}
-
-static int
-sysview_space_execute_update(struct space *space, struct txn *txn,
-			     struct request *request, struct tuple **result)
-{
-	(void)txn;
-	(void)request;
-	(void)result;
-	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
-	return -1;
-}
-
-static int
-sysview_space_execute_upsert(struct space *space, struct txn *txn,
-			     struct request *request)
-{
-	(void)txn;
-	(void)request;
-	diag_set(ClientError, ER_VIEW_IS_RO, space->def->name);
-	return -1;
-}
-
-static struct index *
-sysview_space_create_index(struct space *space, struct index_def *index_def)
-{
-	struct sysview_engine *sysview = (struct sysview_engine *)space->engine;
-	return (struct index *)sysview_index_new(sysview, index_def,
-						 space_name(space));
-}
-
-static const struct space_vtab sysview_space_vtab = {
-	/* .destroy = */ sysview_space_destroy,
-	/* .bsize = */ generic_space_bsize,
-	/* .apply_initial_join_row = */ generic_space_apply_initial_join_row,
-	/* .execute_replace = */ sysview_space_execute_replace,
-	/* .execute_delete = */ sysview_space_execute_delete,
-	/* .execute_update = */ sysview_space_execute_update,
-	/* .execute_upsert = */ sysview_space_execute_upsert,
-	/* .init_system_space = */ generic_init_system_space,
-	/* .check_index_def = */ generic_space_check_index_def,
-	/* .create_index = */ sysview_space_create_index,
-	/* .add_primary_key = */ generic_space_add_primary_key,
-	/* .drop_primary_key = */ generic_space_drop_primary_key,
-	/* .check_format = */ generic_space_check_format,
-	/* .build_index = */ generic_space_build_index,
-	/* .swap_index = */ generic_space_swap_index,
-	/* .prepare_alter = */ generic_space_prepare_alter,
-};
-
-static void
-sysview_engine_shutdown(struct engine *engine)
-{
-	struct sysview_engine *sysview = (struct sysview_engine *)engine;
-	if (mempool_is_initialized(&sysview->iterator_pool))
-		mempool_destroy(&sysview->iterator_pool);
-	free(engine);
-}
-
-static struct space *
-sysview_engine_create_space(struct engine *engine, struct space_def *def,
-			    struct rlist *key_list)
-{
-	struct space *space = (struct space *)calloc(1, sizeof(*space));
-	if (space == NULL) {
-		diag_set(OutOfMemory, sizeof(*space),
-			 "malloc", "struct space");
-		return NULL;
-	}
-	if (space_create(space, engine, &sysview_space_vtab,
-			 def, key_list, NULL) != 0) {
-		free(space);
-		return NULL;
-	}
-	return space;
-}
-
-static const struct engine_vtab sysview_engine_vtab = {
-	/* .shutdown = */ sysview_engine_shutdown,
-	/* .create_space = */ sysview_engine_create_space,
-	/* .join = */ generic_engine_join,
-	/* .begin = */ generic_engine_begin,
-	/* .begin_statement = */ generic_engine_begin_statement,
-	/* .prepare = */ generic_engine_prepare,
-	/* .commit = */ generic_engine_commit,
-	/* .rollback_statement = */ generic_engine_rollback_statement,
-	/* .rollback = */ generic_engine_rollback,
-	/* .bootstrap = */ generic_engine_bootstrap,
-	/* .begin_initial_recovery = */ generic_engine_begin_initial_recovery,
-	/* .begin_final_recovery = */ generic_engine_begin_final_recovery,
-	/* .end_recovery = */ generic_engine_end_recovery,
-	/* .begin_checkpoint = */ generic_engine_begin_checkpoint,
-	/* .wait_checkpoint = */ generic_engine_wait_checkpoint,
-	/* .commit_checkpoint = */ generic_engine_commit_checkpoint,
-	/* .abort_checkpoint = */ generic_engine_abort_checkpoint,
-	/* .collect_garbage = */ generic_engine_collect_garbage,
-	/* .backup = */ generic_engine_backup,
-	/* .memory_stat = */ generic_engine_memory_stat,
-	/* .reset_stat = */ generic_engine_reset_stat,
-	/* .check_space_def = */ generic_engine_check_space_def,
-};
-
-struct sysview_engine *
-sysview_engine_new(void)
-{
-	struct sysview_engine *sysview = calloc(1, sizeof(*sysview));
-	if (sysview == NULL) {
-		diag_set(OutOfMemory, sizeof(*sysview),
-			 "malloc", "struct sysview_engine");
-		return NULL;
-	}
-
-	sysview->base.vtab = &sysview_engine_vtab;
-	sysview->base.name = "sysview";
-	return sysview;
-}
diff --git a/src/box/sysview_index.h b/src/box/sysview_index.h
deleted file mode 100644
index 74f57d4b..00000000
--- a/src/box/sysview_index.h
+++ /dev/null
@@ -1,60 +0,0 @@
-#ifndef TARANTOOL_BOX_SYSVIEW_INDEX_H_INCLUDED
-#define TARANTOOL_BOX_SYSVIEW_INDEX_H_INCLUDED
-/*
- * Copyright 2010-2016, Tarantool AUTHORS, please see AUTHORS file.
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * 1. Redistributions of source code must retain the above
- *    copyright notice, this list of conditions and the
- *    following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above
- *    copyright notice, this list of conditions and the following
- *    disclaimer in the documentation and/or other materials
- *    provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
- * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
- * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
- * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
- * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-#include "index.h"
-
-#if defined(__cplusplus)
-extern "C" {
-#endif /* defined(__cplusplus) */
-
-struct space;
-struct tuple;
-struct sysview_engine;
-
-typedef bool (*sysview_filter_f)(struct space *source, struct tuple *);
-
-struct sysview_index {
-	struct index base;
-	uint32_t source_space_id;
-	uint32_t source_index_id;
-	sysview_filter_f filter;
-};
-
-struct sysview_index *
-sysview_index_new(struct sysview_engine *sysview,
-		  struct index_def *def, const char *space_name);
-
-#if defined(__cplusplus)
-} /* extern "C" */
-#endif /* defined(__cplusplus) */
-
-#endif /* TARANTOOL_BOX_SYSVIEW_INDEX_H_INCLUDED */
-- 
2.11.0

[PATCH 3/6] Rework memtx replace function

[Vladimir Davydov]

From: Nikita Pettik <korablev@tarantool.org>

By now, replace function takes new tuple and old tuple as arguments, instead
of single txn_stmt. It has been done in order to avoid abusing txn_stmt:
the only usage was extracting tuples from it.
As a result, this function can be used by ephemeral tables
without any patching.

(cherry picked from commit 880712c96c00b1667e458e65529b2fd95e99ddc9)
---
 src/box/memtx_space.c | 79 +++++++++++++++++++++++++++++++++------------------
 src/box/memtx_space.h | 20 ++++++-------
 2 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 32700071..26d70482 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -75,11 +75,14 @@ memtx_space_update_bsize(struct space *space,
  * no indexes (is not yet fully built).
  */
 int
-memtx_space_replace_no_keys(struct space *space, struct txn_stmt *stmt,
-			    enum dup_replace_mode mode)
+memtx_space_replace_no_keys(struct space *space, struct tuple *old_tuple,
+			    struct tuple *new_tuple,
+			    enum dup_replace_mode mode, struct tuple **result)
 {
-	(void)stmt;
+	(void)old_tuple;
+	(void)new_tuple;
 	(void)mode;
+	(void)result;
 	struct index *index = index_find(space, 0);
 	assert(index == NULL); /* not reached. */
 	(void) index;
@@ -103,12 +106,15 @@ enum {
  * from snapshot.
  */
 int
-memtx_space_replace_build_next(struct space *space, struct txn_stmt *stmt,
-			       enum dup_replace_mode mode)
+memtx_space_replace_build_next(struct space *space, struct tuple *old_tuple,
+			       struct tuple *new_tuple,
+			       enum dup_replace_mode mode,
+			       struct tuple **result)
 {
-	assert(stmt->old_tuple == NULL && mode == DUP_INSERT);
-	(void) mode;
-	if (stmt->old_tuple) {
+	assert(old_tuple == NULL && mode == DUP_INSERT);
+	(void)mode;
+	*result = NULL;
+	if (old_tuple) {
 		/*
 		 * Called from txn_rollback() In practice
 		 * is impossible: all possible checks for tuple
@@ -118,10 +124,9 @@ memtx_space_replace_build_next(struct space *space, struct txn_stmt *stmt,
 		panic("Failed to commit transaction when loading "
 		      "from snapshot");
 	}
-	if (index_build_next(space->index[0], stmt->new_tuple) != 0)
+	if (index_build_next(space->index[0], new_tuple) != 0)
 		return -1;
-	stmt->engine_savepoint = stmt;
-	memtx_space_update_bsize(space, NULL, stmt->new_tuple);
+	memtx_space_update_bsize(space, NULL, new_tuple);
 	return 0;
 }
 
@@ -130,14 +135,16 @@ memtx_space_replace_build_next(struct space *space, struct txn_stmt *stmt,
  * data from XLOG files.
  */
 int
-memtx_space_replace_primary_key(struct space *space, struct txn_stmt *stmt,
-				enum dup_replace_mode mode)
+memtx_space_replace_primary_key(struct space *space, struct tuple *old_tuple,
+				struct tuple *new_tuple,
+				enum dup_replace_mode mode,
+				struct tuple **result)
 {
-	if (index_replace(space->index[0], stmt->old_tuple,
-			  stmt->new_tuple, mode, &stmt->old_tuple) != 0)
+	if (index_replace(space->index[0], old_tuple,
+			  new_tuple, mode, &old_tuple) != 0)
 		return -1;
-	stmt->engine_savepoint = stmt;
-	memtx_space_update_bsize(space, stmt->old_tuple, stmt->new_tuple);
+	memtx_space_update_bsize(space, old_tuple, new_tuple);
+	*result = old_tuple;
 	return 0;
 }
 
@@ -226,12 +233,12 @@ memtx_space_replace_primary_key(struct space *space, struct txn_stmt *stmt,
  * primary key.
  */
 int
-memtx_space_replace_all_keys(struct space *space, struct txn_stmt *stmt,
-			     enum dup_replace_mode mode)
+memtx_space_replace_all_keys(struct space *space, struct tuple *old_tuple,
+			     struct tuple *new_tuple,
+			     enum dup_replace_mode mode,
+			     struct tuple **result)
 {
 	struct memtx_engine *memtx = (struct memtx_engine *)space->engine;
-	struct tuple *old_tuple = stmt->old_tuple;
-	struct tuple *new_tuple = stmt->new_tuple;
 	/*
 	 * Ensure we have enough slack memory to guarantee
 	 * successful statement-level rollback.
@@ -265,9 +272,8 @@ memtx_space_replace_all_keys(struct space *space, struct txn_stmt *stmt,
 			goto rollback;
 	}
 
-	stmt->old_tuple = old_tuple;
-	stmt->engine_savepoint = stmt;
 	memtx_space_update_bsize(space, old_tuple, new_tuple);
+	*result = old_tuple;
 	return 0;
 
 rollback:
@@ -309,7 +315,8 @@ memtx_space_apply_initial_join_row(struct space *space, struct request *request)
 	if (stmt->new_tuple == NULL)
 		goto rollback;
 	tuple_ref(stmt->new_tuple);
-	if (memtx_space->replace(space, stmt, DUP_INSERT) != 0)
+	if (memtx_space->replace(space, NULL, stmt->new_tuple,
+				 DUP_INSERT, &stmt->old_tuple) != 0)
 		goto rollback;
 	return txn_commit_stmt(txn, request);
 
@@ -331,8 +338,12 @@ memtx_space_execute_replace(struct space *space, struct txn *txn,
 	if (stmt->new_tuple == NULL)
 		return -1;
 	tuple_ref(stmt->new_tuple);
-	if (memtx_space->replace(space, stmt, mode) != 0)
+	struct tuple *old_tuple;
+	if (memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
+				 mode, &old_tuple) != 0)
 		return -1;
+	stmt->old_tuple = old_tuple;
+	stmt->engine_savepoint = stmt;
 	/** The new tuple is referenced by the primary key. */
 	*result = stmt->new_tuple;
 	return 0;
@@ -354,9 +365,13 @@ memtx_space_execute_delete(struct space *space, struct txn *txn,
 		return -1;
 	if (index_get(pk, key, part_count, &stmt->old_tuple) != 0)
 		return -1;
+	struct tuple *old_tuple = NULL;
 	if (stmt->old_tuple != NULL &&
-	    memtx_space->replace(space, stmt, DUP_REPLACE_OR_INSERT) != 0)
+	    memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
+				 DUP_REPLACE_OR_INSERT, &old_tuple) != 0)
 		return -1;
+	stmt->old_tuple = old_tuple;
+	stmt->engine_savepoint = stmt;
 	*result = stmt->old_tuple;
 	return 0;
 }
@@ -399,9 +414,13 @@ memtx_space_execute_update(struct space *space, struct txn *txn,
 	if (stmt->new_tuple == NULL)
 		return -1;
 	tuple_ref(stmt->new_tuple);
+	struct tuple *old_tuple = NULL;
 	if (stmt->old_tuple != NULL &&
-	    memtx_space->replace(space, stmt, DUP_REPLACE) != 0)
+	    memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
+				 DUP_REPLACE, &old_tuple) != 0)
 		return -1;
+	stmt->old_tuple = old_tuple;
+	stmt->engine_savepoint = stmt;
 	*result = stmt->new_tuple;
 	return 0;
 }
@@ -512,9 +531,13 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 	 * we checked this case explicitly and skipped the upsert
 	 * above.
 	 */
+	struct tuple *old_tuple = NULL;
 	if (stmt->new_tuple != NULL &&
-	    memtx_space->replace(space, stmt, DUP_REPLACE_OR_INSERT) != 0)
+	    memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
+				 DUP_REPLACE_OR_INSERT, &old_tuple) != 0)
 		return -1;
+	stmt->old_tuple = old_tuple;
+	stmt->engine_savepoint = stmt;
 	/* Return nothing: UPSERT does not return data. */
 	return 0;
 }
diff --git a/src/box/memtx_space.h b/src/box/memtx_space.h
index 26d33349..7dc34107 100644
--- a/src/box/memtx_space.h
+++ b/src/box/memtx_space.h
@@ -46,8 +46,8 @@ struct memtx_space {
 	 * A pointer to replace function, set to different values
 	 * at different stages of recovery.
 	 */
-	int (*replace)(struct space *, struct txn_stmt *,
-		       enum dup_replace_mode);
+	int (*replace)(struct space *, struct tuple *, struct tuple *,
+		       enum dup_replace_mode, struct tuple **);
 };
 
 /**
@@ -65,17 +65,17 @@ memtx_space_update_bsize(struct space *space,
 			 const struct tuple *new_tuple);
 
 int
-memtx_space_replace_no_keys(struct space *, struct txn_stmt *,
-			    enum dup_replace_mode);
+memtx_space_replace_no_keys(struct space *, struct tuple *, struct tuple *,
+			    enum dup_replace_mode, struct tuple **);
 int
-memtx_space_replace_build_next(struct space *, struct txn_stmt *,
-			       enum dup_replace_mode);
+memtx_space_replace_build_next(struct space *, struct tuple *, struct tuple *,
+			       enum dup_replace_mode, struct tuple **);
 int
-memtx_space_replace_primary_key(struct space *, struct txn_stmt *,
-				enum dup_replace_mode);
+memtx_space_replace_primary_key(struct space *, struct tuple *, struct tuple *,
+				enum dup_replace_mode, struct tuple **);
 int
-memtx_space_replace_all_keys(struct space *, struct txn_stmt *,
-			     enum dup_replace_mode);
+memtx_space_replace_all_keys(struct space *, struct tuple *, struct tuple *,
+			     enum dup_replace_mode, struct tuple **);
 
 struct space *
 memtx_space_new(struct memtx_engine *memtx,
-- 
2.11.0

[PATCH 4/6] txn: unify txn_stmt tuples reference counting rules

[Vladimir Davydov]

Currently, the way txn_stmt::old_tuple and new_tuple are referenced
depends on the engine. For vinyl, the rules are straightforward: if
txn_stmt::{old_tuple,new_tuple} is not NULL, then the reference to the
corresponding tuple is elevated. Hence when a transaction is committed
or rolled back, vinyl calls tuple_unref on both txn_stmt::old_tuple and
new_tuple. For memtx, things are different: the engine doesn't
explicitly increment the reference counter of the tuples - it simply
sets them to the newly inserted tuple and the replaced tuple. On commit,
the reference counter of the old tuple is decreased to delete the
replaced tuple, while on rollback the reference counter of the new tuple
is decreased to delete the new tuple.

Because of this, we can't implement the blackhole engine (aka /dev/null)
without implementing commit and rollback engine methods - even though
such an engine doesn't store anything it still has to set the new_tuple
for on_replace trigger and hence it is responsible for releasing it on
commit or rollback. Since commit/rollback are rather inappropriate for
this kind of engine, let's instead unify txn_stmt reference counting
rules and make txn.c unreference the tuples no matter what engine is.
This doesn't change vinyl, because it already conforms. For memtx, this
means that we need to increase the reference counter when we insert a
new tuple into a space - not a big deal as tuple_ref is almost free.
---
 src/box/memtx_engine.c | 30 ++++++++--------------------
 src/box/memtx_space.c  | 53 ++++++++++++++++++++++++--------------------------
 src/box/txn.c          | 19 ++++++++++++++++++
 src/box/vinyl.c        | 25 ++++--------------------
 4 files changed, 56 insertions(+), 71 deletions(-)

diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index a1ce4fff..f5ace926 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -436,8 +436,9 @@ memtx_engine_rollback_statement(struct engine *engine, struct txn *txn,
 
 	/* Only roll back the changes if they were made. */
 	if (stmt->engine_savepoint == NULL)
-		index_count = 0;
-	else if (memtx_space->replace == memtx_space_replace_all_keys)
+		return;
+
+	if (memtx_space->replace == memtx_space_replace_all_keys)
 		index_count = space->index_count;
 	else if (memtx_space->replace == memtx_space_replace_primary_key)
 		index_count = 1;
@@ -455,16 +456,12 @@ memtx_engine_rollback_statement(struct engine *engine, struct txn *txn,
 			panic("failed to rollback change");
 		}
 	}
-	/** Reset to old bsize, if it was changed. */
-	if (stmt->engine_savepoint != NULL)
-		memtx_space_update_bsize(space, stmt->new_tuple,
-					 stmt->old_tuple);
 
-	if (stmt->new_tuple)
+	memtx_space_update_bsize(space, stmt->new_tuple, stmt->old_tuple);
+	if (stmt->old_tuple != NULL)
+		tuple_ref(stmt->old_tuple);
+	if (stmt->new_tuple != NULL)
 		tuple_unref(stmt->new_tuple);
-
-	stmt->old_tuple = NULL;
-	stmt->new_tuple = NULL;
 }
 
 static void
@@ -480,17 +477,6 @@ memtx_engine_rollback(struct engine *engine, struct txn *txn)
 		memtx_engine_rollback_statement(engine, txn, stmt);
 }
 
-static void
-memtx_engine_commit(struct engine *engine, struct txn *txn)
-{
-	(void)engine;
-	struct txn_stmt *stmt;
-	stailq_foreach_entry(stmt, &txn->stmts, next) {
-		if (stmt->old_tuple)
-			tuple_unref(stmt->old_tuple);
-	}
-}
-
 static int
 memtx_engine_bootstrap(struct engine *engine)
 {
@@ -973,7 +959,7 @@ static const struct engine_vtab memtx_engine_vtab = {
 	/* .begin = */ memtx_engine_begin,
 	/* .begin_statement = */ memtx_engine_begin_statement,
 	/* .prepare = */ memtx_engine_prepare,
-	/* .commit = */ memtx_engine_commit,
+	/* .commit = */ generic_engine_commit,
 	/* .rollback_statement = */ memtx_engine_rollback_statement,
 	/* .rollback = */ memtx_engine_rollback,
 	/* .bootstrap = */ memtx_engine_bootstrap,
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 26d70482..08ae0daa 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -127,6 +127,7 @@ memtx_space_replace_build_next(struct space *space, struct tuple *old_tuple,
 	if (index_build_next(space->index[0], new_tuple) != 0)
 		return -1;
 	memtx_space_update_bsize(space, NULL, new_tuple);
+	tuple_ref(new_tuple);
 	return 0;
 }
 
@@ -144,6 +145,8 @@ memtx_space_replace_primary_key(struct space *space, struct tuple *old_tuple,
 			  new_tuple, mode, &old_tuple) != 0)
 		return -1;
 	memtx_space_update_bsize(space, old_tuple, new_tuple);
+	if (new_tuple != NULL)
+		tuple_ref(new_tuple);
 	*result = old_tuple;
 	return 0;
 }
@@ -273,6 +276,8 @@ memtx_space_replace_all_keys(struct space *space, struct tuple *old_tuple,
 	}
 
 	memtx_space_update_bsize(space, old_tuple, new_tuple);
+	if (new_tuple != NULL)
+		tuple_ref(new_tuple);
 	*result = old_tuple;
 	return 0;
 
@@ -338,11 +343,9 @@ memtx_space_execute_replace(struct space *space, struct txn *txn,
 	if (stmt->new_tuple == NULL)
 		return -1;
 	tuple_ref(stmt->new_tuple);
-	struct tuple *old_tuple;
-	if (memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
-				 mode, &old_tuple) != 0)
+	if (memtx_space->replace(space, NULL, stmt->new_tuple,
+				 mode, &stmt->old_tuple) != 0)
 		return -1;
-	stmt->old_tuple = old_tuple;
 	stmt->engine_savepoint = stmt;
 	/** The new tuple is referenced by the primary key. */
 	*result = stmt->new_tuple;
@@ -363,14 +366,13 @@ memtx_space_execute_delete(struct space *space, struct txn *txn,
 	uint32_t part_count = mp_decode_array(&key);
 	if (exact_key_validate(pk->def->key_def, key, part_count) != 0)
 		return -1;
-	if (index_get(pk, key, part_count, &stmt->old_tuple) != 0)
+	struct tuple *old_tuple;
+	if (index_get(pk, key, part_count, &old_tuple) != 0)
 		return -1;
-	struct tuple *old_tuple = NULL;
-	if (stmt->old_tuple != NULL &&
-	    memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
-				 DUP_REPLACE_OR_INSERT, &old_tuple) != 0)
+	if (old_tuple != NULL &&
+	    memtx_space->replace(space, old_tuple, NULL,
+				 DUP_REPLACE_OR_INSERT, &stmt->old_tuple) != 0)
 		return -1;
-	stmt->old_tuple = old_tuple;
 	stmt->engine_savepoint = stmt;
 	*result = stmt->old_tuple;
 	return 0;
@@ -390,17 +392,18 @@ memtx_space_execute_update(struct space *space, struct txn *txn,
 	uint32_t part_count = mp_decode_array(&key);
 	if (exact_key_validate(pk->def->key_def, key, part_count) != 0)
 		return -1;
-	if (index_get(pk, key, part_count, &stmt->old_tuple) != 0)
+	struct tuple *old_tuple;
+	if (index_get(pk, key, part_count, &old_tuple) != 0)
 		return -1;
 
-	if (stmt->old_tuple == NULL) {
+	if (old_tuple == NULL) {
 		*result = NULL;
 		return 0;
 	}
 
 	/* Update the tuple; legacy, request ops are in request->tuple */
 	uint32_t new_size = 0, bsize;
-	const char *old_data = tuple_data_range(stmt->old_tuple, &bsize);
+	const char *old_data = tuple_data_range(old_tuple, &bsize);
 	const char *new_data =
 		tuple_update_execute(region_aligned_alloc_cb, &fiber()->gc,
 				     request->tuple, request->tuple_end,
@@ -414,12 +417,9 @@ memtx_space_execute_update(struct space *space, struct txn *txn,
 	if (stmt->new_tuple == NULL)
 		return -1;
 	tuple_ref(stmt->new_tuple);
-	struct tuple *old_tuple = NULL;
-	if (stmt->old_tuple != NULL &&
-	    memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
-				 DUP_REPLACE, &old_tuple) != 0)
+	if (memtx_space->replace(space, old_tuple, stmt->new_tuple,
+				 DUP_REPLACE, &stmt->old_tuple) != 0)
 		return -1;
-	stmt->old_tuple = old_tuple;
 	stmt->engine_savepoint = stmt;
 	*result = stmt->new_tuple;
 	return 0;
@@ -453,10 +453,11 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 	mp_decode_array(&key);
 
 	/* Try to find the tuple by primary key. */
-	if (index_get(index, key, part_count, &stmt->old_tuple) != 0)
+	struct tuple *old_tuple;
+	if (index_get(index, key, part_count, &old_tuple) != 0)
 		return -1;
 
-	if (stmt->old_tuple == NULL) {
+	if (old_tuple == NULL) {
 		/**
 		 * Old tuple was not found. A write optimized
 		 * engine may only know this after commit, so
@@ -486,8 +487,7 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 		tuple_ref(stmt->new_tuple);
 	} else {
 		uint32_t new_size = 0, bsize;
-		const char *old_data = tuple_data_range(stmt->old_tuple,
-							&bsize);
+		const char *old_data = tuple_data_range(old_tuple, &bsize);
 		/*
 		 * Update the tuple.
 		 * tuple_upsert_execute() fails on totally wrong
@@ -514,14 +514,13 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 		struct index *pk = space->index[0];
 		if (!key_update_can_be_skipped(pk->def->key_def->column_mask,
 					       column_mask) &&
-		    tuple_compare(stmt->old_tuple, stmt->new_tuple,
+		    tuple_compare(old_tuple, stmt->new_tuple,
 				  pk->def->key_def) != 0) {
 			/* Primary key is changed: log error and do nothing. */
 			diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
 				 pk->def->name, space_name(space));
 			diag_log();
 			tuple_unref(stmt->new_tuple);
-			stmt->old_tuple = NULL;
 			stmt->new_tuple = NULL;
 		}
 	}
@@ -531,12 +530,10 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 	 * we checked this case explicitly and skipped the upsert
 	 * above.
 	 */
-	struct tuple *old_tuple = NULL;
 	if (stmt->new_tuple != NULL &&
-	    memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
-				 DUP_REPLACE_OR_INSERT, &old_tuple) != 0)
+	    memtx_space->replace(space, old_tuple, stmt->new_tuple,
+				 DUP_REPLACE_OR_INSERT, &stmt->old_tuple) != 0)
 		return -1;
-	stmt->old_tuple = old_tuple;
 	stmt->engine_savepoint = stmt;
 	/* Return nothing: UPSERT does not return data. */
 	return 0;
diff --git a/src/box/txn.c b/src/box/txn.c
index cb301015..fbce612a 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -99,6 +99,15 @@ txn_stmt_new(struct txn *txn)
 	return stmt;
 }
 
+static inline void
+txn_stmt_unref_tuples(struct txn_stmt *stmt)
+{
+	if (stmt->old_tuple != NULL)
+		tuple_unref(stmt->old_tuple);
+	if (stmt->new_tuple != NULL)
+		tuple_unref(stmt->new_tuple);
+}
+
 static void
 txn_rollback_to_svp(struct txn *txn, struct stailq_entry *svp)
 {
@@ -116,6 +125,7 @@ txn_rollback_to_svp(struct txn *txn, struct stailq_entry *svp)
 			txn->n_rows--;
 			stmt->row = NULL;
 		}
+		txn_stmt_unref_tuples(stmt);
 	}
 }
 
@@ -325,6 +335,10 @@ txn_commit(struct txn *txn)
 	if (txn->engine != NULL)
 		engine_commit(txn->engine, txn);
 
+	struct txn_stmt *stmt;
+	stailq_foreach_entry(stmt, &txn->stmts, next)
+		txn_stmt_unref_tuples(stmt);
+
 	TRASH(txn);
 	/** Free volatile txn memory. */
 	fiber_gc();
@@ -362,6 +376,11 @@ txn_rollback()
 	}
 	if (txn->engine)
 		engine_rollback(txn->engine, txn);
+
+	struct txn_stmt *stmt;
+	stailq_foreach_entry(stmt, &txn->stmts, next)
+		txn_stmt_unref_tuples(stmt);
+
 	TRASH(txn);
 	/** Free volatile txn memory. */
 	fiber_gc();
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index f02fa638..374c5252 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -2324,17 +2324,6 @@ vinyl_space_execute_upsert(struct space *space, struct txn *txn,
 	return vy_upsert(env, tx, stmt, space, request);
 }
 
-static inline void
-txn_stmt_unref_tuples(struct txn_stmt *stmt)
-{
-	if (stmt->old_tuple)
-		tuple_unref(stmt->old_tuple);
-	if (stmt->new_tuple)
-		tuple_unref(stmt->new_tuple);
-	stmt->old_tuple = NULL;
-	stmt->new_tuple = NULL;
-}
-
 static int
 vinyl_engine_begin(struct engine *engine, struct txn *txn)
 {
@@ -2442,11 +2431,7 @@ vinyl_engine_commit(struct engine *engine, struct txn *txn)
 	/* We can't abort the transaction at this point, use force. */
 	vy_quota_force_use(&env->quota, mem_used_after - mem_used_before);
 
-	struct txn_stmt *stmt;
-	stailq_foreach_entry(stmt, &txn->stmts, next)
-		txn_stmt_unref_tuples(stmt);
 	txn->engine_tx = NULL;
-
 	if (!txn->is_autocommit)
 		trigger_clear(&txn->fiber_on_stop);
 }
@@ -2461,11 +2446,7 @@ vinyl_engine_rollback(struct engine *engine, struct txn *txn)
 
 	vy_tx_rollback(tx);
 
-	struct txn_stmt *stmt;
-	stailq_foreach_entry(stmt, &txn->stmts, next)
-		txn_stmt_unref_tuples(stmt);
 	txn->engine_tx = NULL;
-
 	if (!txn->is_autocommit)
 		trigger_clear(&txn->fiber_on_stop);
 }
@@ -2489,7 +2470,6 @@ vinyl_engine_rollback_statement(struct engine *engine, struct txn *txn,
 	struct vy_tx *tx = txn->engine_tx;
 	assert(tx != NULL);
 	vy_tx_rollback_to_savepoint(tx, stmt->engine_savepoint);
-	txn_stmt_unref_tuples(stmt);
 }
 
 /* }}} Public API of transaction control */
@@ -3290,7 +3270,10 @@ vinyl_space_apply_initial_join_row(struct space *space, struct request *request)
 	else
 		vy_tx_rollback(tx);
 
-	txn_stmt_unref_tuples(&stmt);
+	if (stmt.old_tuple != NULL)
+		tuple_unref(stmt.old_tuple);
+	if (stmt.new_tuple != NULL)
+		tuple_unref(stmt.new_tuple);
 
 	size_t mem_used_after = lsregion_used(&env->mem_env.allocator);
 	assert(mem_used_after >= mem_used_before);
-- 
2.11.0

[PATCH 5/6] space: call before_replace trigger even if space has no indexes

[Vladimir Davydov]

Needed for blackhole spaces (see the next patch), which don't support
indexes per se, but still may have a before_replace trigger installed.
---
 src/box/space.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/box/space.c b/src/box/space.c
index fc59dbe3..871cc670 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -269,23 +269,20 @@ static int
 space_before_replace(struct space *space, struct txn *txn,
 		     struct request *request)
 {
-	if (space->index_count == 0) {
-		/* Empty space, nothing to do. */
-		return 0;
-	}
-
 	struct region *gc = &fiber()->gc;
 	enum iproto_type type = request->type;
-	struct index *pk = space->index[0];
+	struct index *pk = space_index(space, 0);
 
-	const char *key;
-	uint32_t part_count;
-	struct index *index;
+	const char *key = NULL;
+	uint32_t part_count = 0;
+	struct index *index = NULL;
 
 	/*
 	 * Lookup the old tuple.
 	 */
-	if (type == IPROTO_UPDATE || type == IPROTO_DELETE) {
+	switch (type) {
+	case IPROTO_UPDATE:
+	case IPROTO_DELETE:
 		index = index_find_unique(space, request->index_id);
 		if (index == NULL)
 			return -1;
@@ -294,21 +291,27 @@ space_before_replace(struct space *space, struct txn *txn,
 		if (exact_key_validate(index->def->key_def,
 				       key, part_count) != 0)
 			return -1;
-	} else if (type == IPROTO_INSERT || type == IPROTO_REPLACE ||
-		   type == IPROTO_UPSERT) {
+		break;
+	case IPROTO_INSERT:
+	case IPROTO_REPLACE:
+	case IPROTO_UPSERT:
+		if (pk == NULL)
+			break;
 		index = pk;
 		key = tuple_extract_key_raw(request->tuple, request->tuple_end,
 					    index->def->key_def, NULL);
 		if (key == NULL)
 			return -1;
 		part_count = mp_decode_array(&key);
-	} else {
+		break;
+	default:
 		/* Unknown request type, nothing to do. */
 		return 0;
 	}
 
-	struct tuple *old_tuple;
-	if (index_get(index, key, part_count, &old_tuple) != 0)
+	struct tuple *old_tuple = NULL;
+	if (index != NULL &&
+	    index_get(index, key, part_count, &old_tuple) != 0)
 		return -1;
 
 	/*
@@ -419,7 +422,8 @@ space_before_replace(struct space *space, struct txn *txn,
 	 * We don't allow to change the value of the primary key
 	 * in the same statement.
 	 */
-	if (request_changed && old_tuple != NULL && new_tuple != NULL &&
+	if (pk != NULL && request_changed &&
+	    old_tuple != NULL && new_tuple != NULL &&
 	    tuple_compare(old_tuple, new_tuple, pk->def->key_def) != 0) {
 		diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
 			 pk->def->name, space->def->name);
-- 
2.11.0

[PATCH 6/6] Introduce blackhole engine

[Vladimir Davydov]

Blackhole is a very simple engine that allows to create spaces that may
written to, but not read from. It only supports INSERT/REPLACE requests.
It doesn't support any indexes hence SELECT is impossible. It does check
space format though and supports on_replace and before_replace triggers.

The whole purpose of this new engine is writing arbitrary rows to WAL
without storing them anywhere. In particular, we need this engine to
write deferred DELETEs generated for vinyl spaces to WAL.

Needed for #2129

@TarantoolBot document
Title: Document blackhole engine
Spaces created with blackhole engine may be written to, but not read
from (they don't even support space.create_index command). Operations
done on a blackhole space are written to WAL and hence get replicated,
but not stored anywhere else.
---
 src/box/CMakeLists.txt      |   1 +
 src/box/blackhole.c         | 210 ++++++++++++++++++++++++++++++++++++++++++++
 src/box/blackhole.h         |  58 ++++++++++++
 src/box/box.cc              |   4 +
 test/box/blackhole.result   | 177 +++++++++++++++++++++++++++++++++++++
 test/box/blackhole.test.lua |  64 ++++++++++++++
 6 files changed, 514 insertions(+)
 create mode 100644 src/box/blackhole.c
 create mode 100644 src/box/blackhole.h
 create mode 100644 test/box/blackhole.result
 create mode 100644 test/box/blackhole.test.lua

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index fabeb3fd..ad544270 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -69,6 +69,7 @@ add_library(box STATIC
     memtx_engine.c
     memtx_space.c
     sysview.c
+    blackhole.c
     vinyl.c
     vy_stmt.c
     vy_mem.c
diff --git a/src/box/blackhole.c b/src/box/blackhole.c
new file mode 100644
index 00000000..96b9fd14
--- /dev/null
+++ b/src/box/blackhole.c
@@ -0,0 +1,210 @@
+/*
+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "blackhole.h"
+
+#include <stddef.h>
+#include <stdlib.h>
+#include <small/rlist.h>
+
+#include "diag.h"
+#include "error.h"
+#include "errcode.h"
+#include "engine.h"
+#include "space.h"
+#include "txn.h"
+#include "tuple.h"
+#include "xrow.h"
+
+static void
+blackhole_space_destroy(struct space *space)
+{
+	free(space);
+}
+
+static int
+blackhole_space_execute_replace(struct space *space, struct txn *txn,
+				struct request *request, struct tuple **result)
+{
+	struct txn_stmt *stmt = txn_current_stmt(txn);
+	stmt->new_tuple = tuple_new(space->format, request->tuple,
+				    request->tuple_end);
+	if (stmt->new_tuple == NULL)
+		return -1;
+	tuple_ref(stmt->new_tuple);
+	*result = stmt->new_tuple;
+	return 0;
+}
+
+static int
+blackhole_space_execute_delete(struct space *space, struct txn *txn,
+			       struct request *request, struct tuple **result)
+{
+	(void)space;
+	(void)txn;
+	(void)request;
+	(void)result;
+	diag_set(ClientError, ER_UNSUPPORTED, "Blackhole", "delete()");
+	return -1;
+}
+
+static int
+blackhole_space_execute_update(struct space *space, struct txn *txn,
+			       struct request *request, struct tuple **result)
+{
+	(void)space;
+	(void)txn;
+	(void)request;
+	(void)result;
+	diag_set(ClientError, ER_UNSUPPORTED, "Blackhole", "update()");
+	return -1;
+}
+
+static int
+blackhole_space_execute_upsert(struct space *space, struct txn *txn,
+			       struct request *request)
+{
+	(void)space;
+	(void)txn;
+	(void)request;
+	diag_set(ClientError, ER_UNSUPPORTED, "Blackhole", "upsert()");
+	return -1;
+}
+
+static struct index *
+blackhole_space_create_index(struct space *space, struct index_def *def)
+{
+	(void)space;
+	(void)def;
+	/* See blackhole_engine_create_space(). */
+	unreachable();
+	return NULL;
+}
+
+static const struct space_vtab blackhole_space_vtab = {
+	/* .destroy = */ blackhole_space_destroy,
+	/* .bsize = */ generic_space_bsize,
+	/* .apply_initial_join_row = */ generic_space_apply_initial_join_row,
+	/* .execute_replace = */ blackhole_space_execute_replace,
+	/* .execute_delete = */ blackhole_space_execute_delete,
+	/* .execute_update = */ blackhole_space_execute_update,
+	/* .execute_upsert = */ blackhole_space_execute_upsert,
+	/* .init_system_space = */ generic_init_system_space,
+	/* .check_index_def = */ generic_space_check_index_def,
+	/* .create_index = */ blackhole_space_create_index,
+	/* .add_primary_key = */ generic_space_add_primary_key,
+	/* .drop_primary_key = */ generic_space_drop_primary_key,
+	/* .check_format = */ generic_space_check_format,
+	/* .build_index = */ generic_space_build_index,
+	/* .swap_index = */ generic_space_swap_index,
+	/* .prepare_alter = */ generic_space_prepare_alter,
+};
+
+static void
+blackhole_engine_shutdown(struct engine *engine)
+{
+	free(engine);
+}
+
+static struct space *
+blackhole_engine_create_space(struct engine *engine, struct space_def *def,
+			      struct rlist *key_list)
+{
+	if (!rlist_empty(key_list)) {
+		diag_set(ClientError, ER_UNSUPPORTED, "Blackhole", "indexes");
+		return NULL;
+	}
+
+	struct space *space = (struct space *)calloc(1, sizeof(*space));
+	if (space == NULL) {
+		diag_set(OutOfMemory, sizeof(*space),
+			 "malloc", "struct space");
+		return NULL;
+	}
+
+	/* Allocate tuples on runtime arena, but check space format. */
+	struct tuple_format *format;
+	format = tuple_format_new(&tuple_format_runtime->vtab, NULL, 0, 0,
+				  def->fields, def->field_count, def->dict);
+	if (format == NULL) {
+		free(space);
+		return NULL;
+	}
+	format->exact_field_count = def->exact_field_count;
+	tuple_format_ref(format);
+
+	if (space_create(space, engine, &blackhole_space_vtab,
+			 def, key_list, format) != 0) {
+		tuple_format_unref(format);
+		free(space);
+		return NULL;
+	}
+	return space;
+}
+
+static const struct engine_vtab blackhole_engine_vtab = {
+	/* .shutdown = */ blackhole_engine_shutdown,
+	/* .create_space = */ blackhole_engine_create_space,
+	/* .join = */ generic_engine_join,
+	/* .begin = */ generic_engine_begin,
+	/* .begin_statement = */ generic_engine_begin_statement,
+	/* .prepare = */ generic_engine_prepare,
+	/* .commit = */ generic_engine_commit,
+	/* .rollback_statement = */ generic_engine_rollback_statement,
+	/* .rollback = */ generic_engine_rollback,
+	/* .bootstrap = */ generic_engine_bootstrap,
+	/* .begin_initial_recovery = */ generic_engine_begin_initial_recovery,
+	/* .begin_final_recovery = */ generic_engine_begin_final_recovery,
+	/* .end_recovery = */ generic_engine_end_recovery,
+	/* .begin_checkpoint = */ generic_engine_begin_checkpoint,
+	/* .wait_checkpoint = */ generic_engine_wait_checkpoint,
+	/* .commit_checkpoint = */ generic_engine_commit_checkpoint,
+	/* .abort_checkpoint = */ generic_engine_abort_checkpoint,
+	/* .collect_garbage = */ generic_engine_collect_garbage,
+	/* .backup = */ generic_engine_backup,
+	/* .memory_stat = */ generic_engine_memory_stat,
+	/* .reset_stat = */ generic_engine_reset_stat,
+	/* .check_space_def = */ generic_engine_check_space_def,
+};
+
+struct engine *
+blackhole_engine_new(void)
+{
+	struct engine *engine = calloc(1, sizeof(*engine));
+	if (engine == NULL) {
+		diag_set(OutOfMemory, sizeof(*engine),
+			 "malloc", "struct engine");
+		return NULL;
+	}
+
+	engine->vtab = &blackhole_engine_vtab;
+	engine->name = "blackhole";
+	return engine;
+}
diff --git a/src/box/blackhole.h b/src/box/blackhole.h
new file mode 100644
index 00000000..5a78610d
--- /dev/null
+++ b/src/box/blackhole.h
@@ -0,0 +1,58 @@
+#ifndef TARANTOOL_BOX_BLACKHOLE_H_INCLUDED
+#define TARANTOOL_BOX_BLACKHOLE_H_INCLUDED
+/*
+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include <stddef.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct engine *
+blackhole_engine_new(void);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+
+#include "diag.h"
+
+static inline struct engine *
+blackhole_engine_new_xc(void)
+{
+	struct engine *engine = blackhole_engine_new();
+	if (engine == NULL)
+		diag_raise();
+	return engine;
+}
+
+#endif /* defined(__plusplus) */
+
+#endif /* TARANTOOL_BOX_BLACKHOLE_H_INCLUDED */
diff --git a/src/box/box.cc b/src/box/box.cc
index 9c3f87f0..22dc64d4 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -51,6 +51,7 @@
 #include "engine.h"
 #include "memtx_engine.h"
 #include "sysview.h"
+#include "blackhole.h"
 #include "vinyl.h"
 #include "space.h"
 #include "index.h"
@@ -1627,6 +1628,9 @@ engine_init()
 	struct sysview_engine *sysview = sysview_engine_new_xc();
 	engine_register((struct engine *)sysview);
 
+	struct engine *blackhole = blackhole_engine_new_xc();
+	engine_register(blackhole);
+
 	struct vinyl_engine *vinyl;
 	vinyl = vinyl_engine_new_xc(cfg_gets("vinyl_dir"),
 				    cfg_geti64("vinyl_memory"),
diff --git a/test/box/blackhole.result b/test/box/blackhole.result
new file mode 100644
index 00000000..a985fcd7
--- /dev/null
+++ b/test/box/blackhole.result
@@ -0,0 +1,177 @@
+test_run = require('test_run').new()
+---
+...
+s = box.schema.space.create('test', {engine = 'blackhole'})
+---
+...
+-- Blackhole doesn't support indexes.
+s:create_index('pk')
+---
+- error: Blackhole does not support indexes
+...
+-- Blackhole does support space format.
+s:format{{'key', 'unsigned'}, {'value', 'string'}}
+---
+...
+s:format()
+---
+- [{'name': 'key', 'type': 'unsigned'}, {'name': 'value', 'type': 'string'}]
+...
+t = s:insert{1, 'a'} -- ok
+---
+...
+t, t.key, t.value
+---
+- [1, 'a']
+- 1
+- a
+...
+s:insert{1, 2, 3} -- error
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected string'
+...
+s:replace{'a', 'b', 'c'} -- error
+---
+- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+...
+s:format{}
+---
+...
+s:insert{1, 2, 3} -- ok
+---
+- [1, 2, 3]
+...
+s:replace{'a', 'b', 'c'} -- ok
+---
+- ['a', 'b', 'c']
+...
+-- Blackhole doesn't support delete/update/upsert operations.
+box.internal.delete(s.id, 0, {})
+---
+- error: Blackhole does not support delete()
+...
+box.internal.update(s.id, 0, {}, {})
+---
+- error: Blackhole does not support update()
+...
+box.internal.upsert(s.id, {}, {})
+---
+- error: Blackhole does not support upsert()
+...
+-- Blackhole supports on_replace and before_replace triggers.
+s_old = nil
+---
+...
+s_new = nil
+---
+...
+f1 = s:on_replace(function(old, new) s_old = old s_new = new end)
+---
+...
+s:replace{1, 2, 3}
+---
+- [1, 2, 3]
+...
+s_old, s_new
+---
+- null
+- [1, 2, 3]
+...
+f2 = s:before_replace(function(old, new) return box.tuple.new{4, 5, 6} end)
+---
+...
+s:replace{1, 2, 3}
+---
+- [4, 5, 6]
+...
+s_old, s_new
+---
+- null
+- [4, 5, 6]
+...
+s:on_replace(nil, f1)
+---
+...
+s:before_replace(nil, f2)
+---
+...
+-- Test recovery.
+test_run:cmd('restart server default')
+s = box.space.test
+---
+...
+-- Test snapshot.
+box.snapshot()
+---
+- ok
+...
+-- Operations done on a blackhole space are written to the WAL
+-- and therefore get replicated. Check it with the aid of an
+-- on_replace trigger.
+box.schema.user.grant('guest', 'replication')
+---
+...
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+---
+- true
+...
+test_run:cmd("start server replica")
+---
+- true
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+t = {}
+---
+...
+_ = box.space.test:on_replace(function(old, new) table.insert(t, new) end)
+---
+...
+test_run:cmd('switch default')
+---
+- true
+...
+s = box.space.test
+---
+...
+for i = 1, 5 do s:replace{i} end
+---
+...
+vclock = test_run:get_vclock('default')
+---
+...
+test_run:wait_vclock('replica', vclock)
+---
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+t
+---
+- - [1]
+  - [2]
+  - [3]
+  - [4]
+  - [5]
+...
+test_run:cmd('switch default')
+---
+- true
+...
+test_run:cmd("stop server replica")
+---
+- true
+...
+test_run:cmd("cleanup server replica")
+---
+- true
+...
+box.schema.user.revoke('guest', 'replication')
+---
+...
+s:drop()
+---
+...
diff --git a/test/box/blackhole.test.lua b/test/box/blackhole.test.lua
new file mode 100644
index 00000000..5dcf9e61
--- /dev/null
+++ b/test/box/blackhole.test.lua
@@ -0,0 +1,64 @@
+test_run = require('test_run').new()
+
+s = box.schema.space.create('test', {engine = 'blackhole'})
+
+-- Blackhole doesn't support indexes.
+s:create_index('pk')
+
+-- Blackhole does support space format.
+s:format{{'key', 'unsigned'}, {'value', 'string'}}
+s:format()
+t = s:insert{1, 'a'} -- ok
+t, t.key, t.value
+s:insert{1, 2, 3} -- error
+s:replace{'a', 'b', 'c'} -- error
+s:format{}
+s:insert{1, 2, 3} -- ok
+s:replace{'a', 'b', 'c'} -- ok
+
+-- Blackhole doesn't support delete/update/upsert operations.
+box.internal.delete(s.id, 0, {})
+box.internal.update(s.id, 0, {}, {})
+box.internal.upsert(s.id, {}, {})
+
+-- Blackhole supports on_replace and before_replace triggers.
+s_old = nil
+s_new = nil
+f1 = s:on_replace(function(old, new) s_old = old s_new = new end)
+s:replace{1, 2, 3}
+s_old, s_new
+f2 = s:before_replace(function(old, new) return box.tuple.new{4, 5, 6} end)
+s:replace{1, 2, 3}
+s_old, s_new
+s:on_replace(nil, f1)
+s:before_replace(nil, f2)
+
+-- Test recovery.
+test_run:cmd('restart server default')
+s = box.space.test
+
+-- Test snapshot.
+box.snapshot()
+
+-- Operations done on a blackhole space are written to the WAL
+-- and therefore get replicated. Check it with the aid of an
+-- on_replace trigger.
+box.schema.user.grant('guest', 'replication')
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+test_run:cmd("start server replica")
+test_run:cmd("switch replica")
+t = {}
+_ = box.space.test:on_replace(function(old, new) table.insert(t, new) end)
+test_run:cmd('switch default')
+s = box.space.test
+for i = 1, 5 do s:replace{i} end
+vclock = test_run:get_vclock('default')
+test_run:wait_vclock('replica', vclock)
+test_run:cmd("switch replica")
+t
+test_run:cmd('switch default')
+test_run:cmd("stop server replica")
+test_run:cmd("cleanup server replica")
+box.schema.user.revoke('guest', 'replication')
+
+s:drop()
-- 
2.11.0

Re: [PATCH 1/6] Add generic engine, space, index method stubs

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> This should reduce maintenance burden and help us introduce a new
> engine.
> ---
>  src/box/engine.c         | 161 ++++++++++++++++++++++++++
>  src/box/engine.h         |  28 +++++
>  src/box/index.cc         |  13 +++
>  src/box/index.h          |   3 +
>  src/box/memtx_engine.c   |  17 +--
>  src/box/space.c          |  73 ++++++++++++
>  src/box/space.h          |  14 +++
>  src/box/sysview_engine.c | 287 +++++------------------------------------------

OK to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 2/6] Merge sysview_index.[hc] and sysview_engine.[hc]

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> They are fairly small and closely related so let's merge them and call
> the result sysview.[hc].

OK to push.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 3/6] Rework memtx replace function

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:

> By now, replace function takes new tuple and old tuple as arguments, instead
> of single txn_stmt. It has been done in order to avoid abusing txn_stmt:
> the only usage was extracting tuples from it.
> As a result, this function can be used by ephemeral tables
> without any patching.

OK to push.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 4/6] txn: unify txn_stmt tuples reference counting rules

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:

Back in the times of 1.4 it was considered a significant
optimization...

It's a bit difficult to check some minor changes of this patch in
the diff.

OK to push.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 5/6] space: call before_replace trigger even if space has no indexes

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> Needed for blackhole spaces (see the next patch), which don't support
> indexes per se, but still may have a before_replace trigger installed.

Why do you need to enable before replace triggers for blackhole?
What's wrong with on_replace triggers?
 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 6/6] Introduce blackhole engine

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> Blackhole is a very simple engine that allows to create spaces that may
> written to, but not read from. It only supports INSERT/REPLACE requests.
> It doesn't support any indexes hence SELECT is impossible. It does check
> space format though and supports on_replace and before_replace triggers.

Allowing DELETE requests could be practical in the future.
> 
> The whole purpose of this new engine is writing arbitrary rows to WAL
> without storing them anywhere. In particular, we need this engine to
> write deferred DELETEs generated for vinyl spaces to WAL.

This is OK to push.
> 
> Needed for #2129
> 
> @TarantoolBot document
> Title: Document blackhole engine
> Spaces created with blackhole engine may be written to, but not read
> from (they don't even support space.create_index command). Operations
> done on a blackhole space are written to WAL and hence get replicated,
> but not stored anywhere else.

The docbot comment is insufficient. Please describe the effect of
adding triggers to this engine. It should be OK to mix this
engine with others in transaction - please could write a follow
up which allows this? 

Please document how it could be used for logical replication.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 1/6] Add generic engine, space, index method stubs

[Vladimir Davydov]

Pushed to 1.10

Re: [PATCH 2/6] Merge sysview_index.[hc] and sysview_engine.[hc]

[Vladimir Davydov]

Pushed to 1.10

Re: [PATCH 3/6] Rework memtx replace function

[Vladimir Davydov]

Pushed to 1.10

Re: [PATCH 4/6] txn: unify txn_stmt tuples reference counting rules

[Vladimir Davydov]

Pushed to 1.10

Re: [PATCH 5/6] space: call before_replace trigger even if space has no indexes

[Vladimir Davydov]

On Fri, Jul 20, 2018 at 09:39:09PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> > Needed for blackhole spaces (see the next patch), which don't support
> > indexes per se, but still may have a before_replace trigger installed.
> 
> Why do you need to enable before replace triggers for blackhole?

Why not? before_replace can be installed on a blackhole space so
I guess it should work, otherwise it may raise questions.

> What's wrong with on_replace triggers?

Nothing wrong at all. They work as well.

Re: [PATCH 6/6] Introduce blackhole engine

[Vladimir Davydov]

On Fri, Jul 20, 2018 at 09:42:47PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> > Blackhole is a very simple engine that allows to create spaces that may
> > written to, but not read from. It only supports INSERT/REPLACE requests.
> > It doesn't support any indexes hence SELECT is impossible. It does check
> > space format though and supports on_replace and before_replace triggers.
> 
> Allowing DELETE requests could be practical in the future.

True. E.g. MySQL uses the blackhole engine for implementing a kind of
replication filters:

  https://dev.mysql.com/doc/refman/8.0/en/blackhole-storage-engine.html

Using the blackhole engine like that would require supporting all
available DML requests. However, implementing DELETE/UPDATE implies
index support, which is beyond the scope of this patch. We may do it
later if we need to.

> > 
> > The whole purpose of this new engine is writing arbitrary rows to WAL
> > without storing them anywhere. In particular, we need this engine to
> > write deferred DELETEs generated for vinyl spaces to WAL.
> 
> This is OK to push.
> > 
> > Needed for #2129
> > 
> > @TarantoolBot document
> > Title: Document blackhole engine
> > Spaces created with blackhole engine may be written to, but not read
> > from (they don't even support space.create_index command). Operations
> > done on a blackhole space are written to WAL and hence get replicated,
> > but not stored anywhere else.
> 
> The docbot comment is insufficient. Please describe the effect of
> adding triggers to this engine. It should be OK to mix this
> engine with others in transaction - please could write a follow
> up which allows this? 

I prepared a patch that allows to mix sysview requests in other engines'
transactions, see

  https://www.freelists.org/post/tarantool-patches/PATCH-tx-exclude-sysview-engine-from-transaction-control

Please take a look at it. If it's OK, then all we'll need to do is set
ENGINE_BYPASS_TX flag for the blackhole engine.

> 
> Please document how it could be used for logical replication.

Will do.

Re: [PATCH 5/6] space: call before_replace trigger even if space has no indexes

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/21 23:52]:
> On Fri, Jul 20, 2018 at 09:39:09PM +0300, Konstantin Osipov wrote:
> > * Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> > > Needed for blackhole spaces (see the next patch), which don't support
> > > indexes per se, but still may have a before_replace trigger installed.
> > 
> > Why do you need to enable before replace triggers for blackhole?
> 
> Why not? before_replace can be installed on a blackhole space so
> I guess it should work, otherwise it may raise questions.
> 
> > What's wrong with on_replace triggers?
> 
> Nothing wrong at all. They work as well.

OK, maybe I'm a performance junkie, but I saw you had to
pessimize before_replace to add support for blackhole, and thought
for a minute that for the task at hand you could get by with
    on_replace(). Now I realize your patch is off the hot path,
    before_replace is expensive because it makes a lookup, not
    because there's a few branches here and there.

Thanks, it's OK to push this patch as well then. I assume you will
be adding a test case for space-with-no-indexes in a later patch?

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 5/6] space: call before_replace trigger even if space has no indexes

[Vladimir Davydov]

On Sat, Jul 21, 2018 at 11:59:48PM +0300, Konstantin Osipov wrote:
> Thanks, it's OK to push this patch as well then. I assume you will
> be adding a test case for space-with-no-indexes in a later patch?

Correct, it's in patch 6. I pushed this one to 1.10.

Re: [PATCH 6/6] Introduce blackhole engine

[Vladimir Davydov]

On Sat, Jul 21, 2018 at 03:35:31PM +0300, Vladimir Davydov wrote:
> > Please document how it could be used for logical replication.
> 
> Will do.

Come to think of it, using a blackhole space for replicating CALLs with
the weaponry we have at hand isn't as simple as I thought it would be.
The problem is we need to receive the snapshot when a replica is joined,
but then unsubscribe from all changes except those done to the blackhole
space that is used for logging CALLs. That is, we can't just mark all
spaces except DD as replica local, because they won't be sent in a
snapshot then. The only solution I can come up with now is using
before_replace trigger for filtering out DML requests received from the
master, but this isn't the best practice, because before_replace trigger
is somewhat slow.

If we had replication groups things would be different though. Logical
replication would consist of the following steps then:

1. Create a new replication group on the master. Use the new group for
   creating all user spaces.
2. Create a blackhole space for logging CALLs. Do NOT include it in the
   new replication group (mark it global).
3. Join a replica to the master as a member of the replication group
   created on step 1 so that it receives the whole user data set in a
   snapshot.
4. Stop replication temporarily.
5. Install a trigger on the space used for logging CALLs.
6. Remove the replica from the replication group so that it won't
   receive any changes from the master except CALL log and resume
   replication.

   (ideally, the process should be automated by tarantool as much as
   possible)

That said, I don't think it's worth exposing blackhole to the user until
we have replication groups and some guidelines for logical replication.
What about leaving blackhole undocumented for now to prevent users from
abusing it in ways that we haven't thought through?

Re: [PATCH 6/6] Introduce blackhole engine

[Vladimir Davydov]

Pushed to 1.10 without a documentation request.

(
  Kostja agreed (grudgingly) to leave blackhole engine undocumented
  until we develop a guideline describing how to use this engine for
  logical replication. The latter may require implementing other
  server features, such as replication groups.
)

Re: [PATCH 0/6] Introduce blackhole engine

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/20 20:45]:
> In the scope of #2129, we need to store deferred DELETE statements
> generated during primary index compaction in the WAL so that we can
> recover them after restart. This patch set introduces a new engine for
> this purpose - blackhole. Spaces that use this engine may be written to,
> but not read from (they don't even support creation of indexes). All
> operations done on a blackhole space are logged in the WAL and hence
> replayed during recovery, which is just what we need.
> 
> https://github.com/tarantool/tarantool/issues/2129
> https://github.com/tarantool/tarantool/commits/dv/blackhole

This patch stack has been pushed.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov