[PATCH v2 0/4] Reuse tuple formats for ephemeral spaces

[PATCH v2 0/4] Reuse tuple formats for ephemeral spaces

[Kirill Yukhin]

This patchset makes it possible to re-use tuple_format around
multiple ephemeral spaces. This is needed when many SQL requests
are served at the same time. In this case for almost each non-trivial
SELECT request ephemeral space is created with its own format.
Since removal of ephemeral spaces doesn't mean removal of format,
and formats count is limited, it is pretty much simple to run out of
formats.
This occurs on TPC-C benchmark, when number of clients > 32.

Issue https://github.com/tarantool/tarantool/issues/3924
Branch https://github.com/tarantool/tarantool/commits/kyukhin/gh-3924-reuse-eph-formats

Kirill Yukhin (4):
  Pass necessary fields to tuple_format contructor
  Set is_temporary flag for formats of ephemeral spaces
  sql: set error type in case of ephemral space creation failure
  Allow to reuse tuple_formats for ephemeral spaces

ChangeLog:
  - Fixed all nits pointed
  - Hashing of formats refactored: extracted into dedicated
    routine()
  - Moved initialization of hasher to tuple_init()
  - Removed dead code from test.

 src/box/blackhole.c             |   7 +-
 src/box/box.cc                  |   1 +
 src/box/memtx_engine.c          |   6 ++
 src/box/memtx_space.c           |   9 +-
 src/box/space.c                 |   3 +-
 src/box/space_def.c             |  16 +++
 src/box/space_def.h             |  14 +++
 src/box/sql.c                   |   6 +-
 src/box/sql/vdbe.c              |   4 +-
 src/box/tuple.c                 |  13 ++-
 src/box/tuple_format.c          | 184 +++++++++++++++++++++++++++++---
 src/box/tuple_format.h          |  31 +++++-
 src/box/vinyl.c                 |  12 ++-
 src/box/vy_lsm.c                |  10 +-
 src/errinj.h                    |   2 +
 test/box/errinj.result          |   8 +-
 test/sql/errinj.result          |  36 +++++++
 test/sql/errinj.test.lua        |  13 +++
 test/unit/tuple_bigref.c        |   1 +
 test/unit/vy_iterators_helper.c |  14 +--
 test/unit/vy_mem.c              |   5 +-
 test/unit/vy_point_lookup.c     |   5 +-
 22 files changed, 342 insertions(+), 58 deletions(-)

-- 
2.19.1

[PATCH v2 1/4] Pass necessary fields to tuple_format contructor

[Kirill Yukhin]

There were three extra fields of tuple_format which were setup
after it was created. Fix that by extending tuple_format
contstructor w/ three new arguments: engine, is_temporary,
exact_field_count.
---
 src/box/blackhole.c             |  6 +++---
 src/box/memtx_space.c           |  9 ++++-----
 src/box/tuple.c                 |  8 ++++----
 src/box/tuple_format.c          | 13 ++++++++-----
 src/box/tuple_format.h          | 11 ++++++++---
 src/box/vinyl.c                 | 11 ++++++-----
 src/box/vy_lsm.c                |  9 +++++----
 test/unit/vy_iterators_helper.c | 12 ++++++------
 test/unit/vy_mem.c              |  4 ++--
 test/unit/vy_point_lookup.c     |  4 ++--
 10 files changed, 48 insertions(+), 39 deletions(-)

diff --git a/src/box/blackhole.c b/src/box/blackhole.c
index 0412ffe..6ada7a5 100644
--- a/src/box/blackhole.c
+++ b/src/box/blackhole.c
@@ -155,13 +155,13 @@ blackhole_engine_create_space(struct engine *engine, struct space_def *def,
 
 	/* Allocate tuples on runtime arena, but check space format. */
 	struct tuple_format *format;
-	format = tuple_format_new(&tuple_format_runtime->vtab, NULL, 0,
-				  def->fields, def->field_count, def->dict);
+	format = tuple_format_new(&tuple_format_runtime->vtab, NULL, NULL, 0,
+				  def->fields, def->field_count,
+				  def->exact_field_count, def->dict, false);
 	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,
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index eb790a6..4aad8da 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -990,15 +990,14 @@ memtx_space_new(struct memtx_engine *memtx,
 		keys[key_count++] = index_def->key_def;
 
 	struct tuple_format *format =
-		tuple_format_new(&memtx_tuple_format_vtab, keys, key_count,
-				 def->fields, def->field_count, def->dict);
+		tuple_format_new(&memtx_tuple_format_vtab, memtx, keys, key_count,
+				 def->fields, def->field_count,
+				 def->exact_field_count, def->dict,
+				 def->opts.is_temporary);
 	if (format == NULL) {
 		free(memtx_space);
 		return NULL;
 	}
-	format->engine = memtx;
-	format->is_temporary = def->opts.is_temporary;
-	format->exact_field_count = def->exact_field_count;
 	tuple_format_ref(format);
 
 	if (space_create((struct space *)memtx_space, (struct engine *)memtx,
diff --git a/src/box/tuple.c b/src/box/tuple.c
index 2343f0e..a87b2bd 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -207,8 +207,8 @@ tuple_init(field_name_hash_f hash)
 	/*
 	 * Create a format for runtime tuples
 	 */
-	tuple_format_runtime = tuple_format_new(&tuple_format_runtime_vtab,
-						NULL, 0, NULL, 0, NULL);
+	tuple_format_runtime = tuple_format_new(&tuple_format_runtime_vtab, NULL,
+						NULL, 0, NULL, 0, 0, NULL, false);
 	if (tuple_format_runtime == NULL)
 		return -1;
 
@@ -379,8 +379,8 @@ box_tuple_format_t *
 box_tuple_format_new(struct key_def **keys, uint16_t key_count)
 {
 	box_tuple_format_t *format =
-		tuple_format_new(&tuple_format_runtime_vtab,
-				 keys, key_count, NULL, 0, NULL);
+		tuple_format_new(&tuple_format_runtime_vtab, NULL,
+				 keys, key_count, NULL, 0, 0, NULL, false);
 	if (format != NULL)
 		tuple_format_ref(format);
 	return format;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index e11b4e6..559c601 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -398,17 +398,20 @@ tuple_format_delete(struct tuple_format *format)
 }
 
 struct tuple_format *
-tuple_format_new(struct tuple_format_vtab *vtab, struct key_def * const *keys,
-		 uint16_t key_count, const struct field_def *space_fields,
-		 uint32_t space_field_count, struct tuple_dictionary *dict)
+tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
+		 struct key_def * const *keys, uint16_t key_count,
+		 const struct field_def *space_fields,
+		 uint32_t space_field_count, uint32_t exact_field_count,
+		 struct tuple_dictionary *dict, bool is_temporary)
 {
 	struct tuple_format *format =
 		tuple_format_alloc(keys, key_count, space_field_count, dict);
 	if (format == NULL)
 		return NULL;
 	format->vtab = *vtab;
-	format->engine = NULL;
-	format->is_temporary = false;
+	format->engine = engine;
+	format->is_temporary = is_temporary;
+	format->exact_field_count = exact_field_count;
 	if (tuple_format_register(format) < 0) {
 		tuple_format_destroy(format);
 		free(format);
diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
index 30b93b6..d15fef2 100644
--- a/src/box/tuple_format.h
+++ b/src/box/tuple_format.h
@@ -258,18 +258,23 @@ tuple_format_unref(struct tuple_format *format)
 /**
  * Allocate, construct and register a new in-memory tuple format.
  * @param vtab Virtual function table for specific engines.
+ * @param engine Pointer to storage engine.
  * @param keys Array of key_defs of a space.
  * @param key_count The number of keys in @a keys array.
  * @param space_fields Array of fields, defined in a space format.
  * @param space_field_count Length of @a space_fields.
+ * @param exact_field_count Exact field count for format.
+ * @param is_temporary Set if format belongs to temporary space.
  *
  * @retval not NULL Tuple format.
  * @retval     NULL Memory error.
  */
 struct tuple_format *
-tuple_format_new(struct tuple_format_vtab *vtab, struct key_def * const *keys,
-		 uint16_t key_count, const struct field_def *space_fields,
-		 uint32_t space_field_count, struct tuple_dictionary *dict);
+tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
+		 struct key_def * const *keys, uint16_t key_count,
+		 const struct field_def *space_fields,
+		 uint32_t space_field_count, uint32_t exact_field_count,
+		 struct tuple_dictionary *dict, bool is_temporary);
 
 /**
  * Check, if @a format1 can store any tuples of @a format2. For
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index d6117f4..49e8372 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -619,13 +619,13 @@ vinyl_engine_create_space(struct engine *engine, struct space_def *def,
 		keys[key_count++] = index_def->key_def;
 
 	struct tuple_format *format =
-		tuple_format_new(&vy_tuple_format_vtab, keys, key_count,
-				 def->fields, def->field_count, def->dict);
+		tuple_format_new(&vy_tuple_format_vtab, NULL, keys, key_count,
+				 def->fields, def->field_count,
+				 def->exact_field_count, def->dict, false);
 	if (format == NULL) {
 		free(space);
 		return NULL;
 	}
-	format->exact_field_count = def->exact_field_count;
 	tuple_format_ref(format);
 
 	if (space_create(space, engine, &vinyl_space_vtab,
@@ -3043,8 +3043,9 @@ vy_send_lsm(struct vy_join_ctx *ctx, struct vy_lsm_recovery_info *lsm_info)
 				   lsm_info->key_part_count);
 	if (ctx->key_def == NULL)
 		goto out;
-	ctx->format = tuple_format_new(&vy_tuple_format_vtab, &ctx->key_def,
-				       1, NULL, 0, NULL);
+	ctx->format = tuple_format_new(&vy_tuple_format_vtab, NULL,
+				       &ctx->key_def, 1, NULL, 0, 0, NULL,
+				       false);
 	if (ctx->format == NULL)
 		goto out_free_key_def;
 	tuple_format_ref(ctx->format);
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 5eb3fd7..7a6ba6e 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -60,8 +60,8 @@ vy_lsm_env_create(struct vy_lsm_env *env, const char *path,
 		  vy_upsert_thresh_cb upsert_thresh_cb,
 		  void *upsert_thresh_arg)
 {
-	env->key_format = tuple_format_new(&vy_tuple_format_vtab,
-					   NULL, 0, NULL, 0, NULL);
+	env->key_format = tuple_format_new(&vy_tuple_format_vtab, NULL,
+					   NULL, 0, NULL, 0, 0, NULL, false);
 	if (env->key_format == NULL)
 		return -1;
 	tuple_format_ref(env->key_format);
@@ -153,8 +153,9 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 		 */
 		lsm->disk_format = format;
 	} else {
-		lsm->disk_format = tuple_format_new(&vy_tuple_format_vtab,
-						    &cmp_def, 1, NULL, 0, NULL);
+		lsm->disk_format = tuple_format_new(&vy_tuple_format_vtab, NULL,
+						    &cmp_def, 1, NULL, 0, 0,
+						    NULL, false);
 		if (lsm->disk_format == NULL)
 			goto fail_format;
 	}
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index 7fad560..6808ff4 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -21,8 +21,8 @@ vy_iterator_C_test_init(size_t cache_size)
 	tuple_init(NULL);
 	vy_cache_env_create(&cache_env, cord_slab_cache());
 	vy_cache_env_set_quota(&cache_env, cache_size);
-	vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, 0,
-					 NULL, 0, NULL);
+	vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, NULL, 0,
+					 NULL, 0, 0, NULL, false);
 	tuple_format_ref(vy_key_format);
 
 	size_t mem_size = 64 * 1024 * 1024;
@@ -201,8 +201,8 @@ create_test_mem(struct key_def *def)
 	/* Create format */
 	struct key_def * const defs[] = { def };
 	struct tuple_format *format =
-		tuple_format_new(&vy_tuple_format_vtab, defs, def->part_count,
-				 NULL, 0, NULL);
+		tuple_format_new(&vy_tuple_format_vtab, NULL, defs,
+				 def->part_count, NULL, 0, 0, NULL, false);
 	fail_if(format == NULL);
 
 	/* Create mem */
@@ -219,8 +219,8 @@ create_test_cache(uint32_t *fields, uint32_t *types,
 	*def = box_key_def_new(fields, types, key_cnt);
 	assert(*def != NULL);
 	vy_cache_create(cache, &cache_env, *def, true);
-	*format = tuple_format_new(&vy_tuple_format_vtab, def, 1, NULL, 0,
-				   NULL);
+	*format = tuple_format_new(&vy_tuple_format_vtab, NULL, def, 1, NULL, 0,
+				   0, NULL, false);
 	tuple_format_ref(*format);
 }
 
diff --git a/test/unit/vy_mem.c b/test/unit/vy_mem.c
index ebf3fbc..a13c58b 100644
--- a/test/unit/vy_mem.c
+++ b/test/unit/vy_mem.c
@@ -77,8 +77,8 @@ test_iterator_restore_after_insertion()
 
 	/* Create format */
 	struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
-						       &key_def, 1, NULL, 0,
-						       NULL);
+						       NULL, &key_def, 1, NULL,
+						       0, 0, NULL, false);
 	assert(format != NULL);
 	tuple_format_ref(format);
 
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index 65dafcb..c19d307 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -84,8 +84,8 @@ test_basic()
 
 	vy_cache_create(&cache, &cache_env, key_def, true);
 	struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
-						       &key_def, 1, NULL, 0,
-						       NULL);
+						       NULL, &key_def, 1, NULL,
+						       0, 0, NULL, false);
 	isnt(format, NULL, "tuple_format_new is not NULL");
 	tuple_format_ref(format);
 
-- 
2.19.1

[PATCH v2 2/4] Set is_temporary flag for formats of ephemeral spaces

[Kirill Yukhin]

Before the patch, when ephemeral space was created flag
is_temporary was set after space was actually created.
Which in turn lead to corresponding flag of tuple_format
being set to `false`.
So, having heavy load using ephemeral spaces (almost any
SQL query) and snapshotting at the same time might lead
to OOM, since tuples of ephemeral spaces were not marked
as temporary and were not gc-ed.
Patch sets the flag in space definition.
---
 src/box/space.c     |  2 +-
 src/box/space_def.c | 14 ++++++++++++++
 src/box/space_def.h |  9 +++++++++
 src/box/sql.c       |  6 +-----
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/box/space.c b/src/box/space.c
index 4d174f7..6245167 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -194,10 +194,10 @@ space_new(struct space_def *def, struct rlist *key_list)
 struct space *
 space_new_ephemeral(struct space_def *def, struct rlist *key_list)
 {
+	assert(def->opts.is_temporary);
 	struct space *space = space_new(def, key_list);
 	if (space == NULL)
 		return NULL;
-	space->def->opts.is_temporary = true;
 	space->vtab->init_ephemeral_space(space);
 	return space;
 }
diff --git a/src/box/space_def.c b/src/box/space_def.c
index 3516bdd..d60c2d3 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -257,6 +257,20 @@ space_def_new(uint32_t id, uint32_t uid, uint32_t exact_field_count,
 	return def;
 }
 
+struct space_def*
+space_def_new_ephemeral(uint32_t field_count)
+{
+	struct space_opts opts = space_opts_default;
+	opts.is_temporary = true;
+	struct space_def *space_def = space_def_new(0, 0, field_count,
+						    "ephemeral",
+						    strlen("ephemeral"),
+						    "memtx", strlen("memtx"),
+						    &opts, &field_def_default,
+						    0);
+	return space_def;
+}
+
 /** Free a default value's syntax trees of @a defs. */
 void
 space_def_destroy_fields(struct field_def *fields, uint32_t field_count,
diff --git a/src/box/space_def.h b/src/box/space_def.h
index 8044f88..b6ab6b3 100644
--- a/src/box/space_def.h
+++ b/src/box/space_def.h
@@ -169,6 +169,15 @@ space_def_new(uint32_t id, uint32_t uid, uint32_t exact_field_count,
 	      const struct space_opts *opts, const struct field_def *fields,
 	      uint32_t field_count);
 
+/**
+ * Create a new ephemeral space definition.
+ * @param field_count Number of fields in the space.
+ *
+ * @retval Space definition.
+ */
+struct space_def *
+space_def_new_ephemeral(uint32_t field_count);
+
 /**
  * Size of the space_def.
  * @param name_len Length of the space name.
diff --git a/src/box/sql.c b/src/box/sql.c
index 081a038..387da7b 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -408,11 +408,7 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
 	rlist_add_entry(&key_list, ephemer_index_def, link);
 
 	struct space_def *ephemer_space_def =
-		space_def_new(0 /* space id */, 0 /* user id */, field_count,
-			      "ephemeral", strlen("ephemeral"),
-			      "memtx", strlen("memtx"),
-			      &space_opts_default, &field_def_default,
-			      0 /* length of field_def */);
+		space_def_new_ephemeral(field_count);
 	if (ephemer_space_def == NULL) {
 		index_def_delete(ephemer_index_def);
 		return NULL;
-- 
2.19.1

[PATCH v2 3/4] sql: set error type in case of ephemral space creation failure

[Kirill Yukhin]

This is trivial patch which sets error kind if epehemeral
spaces cannot be created due to Tarantool's backend (e.g. there's
no more memory or formats available).
---
 src/box/sql/vdbe.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9f9d0ea..9fc362f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3272,8 +3272,10 @@ case OP_OpenTEphemeral: {
 	struct space *space = sql_ephemeral_space_create(pOp->p2,
 							 pOp->p4.key_info);
 
-	if (space == NULL)
+	if (space == NULL) {
+		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
+	}
 	aMem[pOp->p1].u.p = space;
 	aMem[pOp->p1].flags = MEM_Ptr;
 	break;
-- 
2.19.1

[PATCH v2 4/4] Allow to reuse tuple_formats for ephemeral spaces

[Kirill Yukhin]

Since under heavy load with SQL queries ephemeral
spaces might be extensively used it is possible to run out
of tuple_formats for such spaces. This occurs because
tuple_format is not immediately deleted when ephemeral space is
dropped. Its removel is postponed instead and triggered only
when tuple memory is exhausted.
As far as there's no way to alter ephemeral space's format,
let's re-use them for multiple epehemral spaces in case
they're identical.

Closes #3924
---
 src/box/blackhole.c             |   3 +-
 src/box/box.cc                  |   1 +
 src/box/memtx_engine.c          |   6 ++
 src/box/memtx_space.c           |   2 +-
 src/box/space.c                 |   1 +
 src/box/space_def.c             |   2 +
 src/box/space_def.h             |   5 +
 src/box/tuple.c                 |   9 +-
 src/box/tuple_format.c          | 173 ++++++++++++++++++++++++++++++--
 src/box/tuple_format.h          |  22 +++-
 src/box/vinyl.c                 |   5 +-
 src/box/vy_lsm.c                |   5 +-
 src/errinj.h                    |   2 +
 test/box/errinj.result          |   8 +-
 test/sql/errinj.result          |  36 +++++++
 test/sql/errinj.test.lua        |  13 +++
 test/unit/tuple_bigref.c        |   1 +
 test/unit/vy_iterators_helper.c |   8 +-
 test/unit/vy_mem.c              |   3 +-
 test/unit/vy_point_lookup.c     |   3 +-
 20 files changed, 281 insertions(+), 27 deletions(-)

diff --git a/src/box/blackhole.c b/src/box/blackhole.c
index 6ada7a5..249eb67 100644
--- a/src/box/blackhole.c
+++ b/src/box/blackhole.c
@@ -157,7 +157,8 @@ blackhole_engine_create_space(struct engine *engine, struct space_def *def,
 	struct tuple_format *format;
 	format = tuple_format_new(&tuple_format_runtime->vtab, NULL, NULL, 0,
 				  def->fields, def->field_count,
-				  def->exact_field_count, def->dict, false);
+				  def->exact_field_count, def->dict, false,
+				  false);
 	if (format == NULL) {
 		free(space);
 		return NULL;
diff --git a/src/box/box.cc b/src/box/box.cc
index 9f2fd6d..ae7f471 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -46,6 +46,7 @@
 #include <rmean.h>
 #include "main.h"
 #include "tuple.h"
+#include "tuple_format.h"
 #include "session.h"
 #include "schema.h"
 #include "engine.h"
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index 5cf70ab..254ef24 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -994,6 +994,12 @@ memtx_engine_gc_f(va_list va)
 	struct memtx_engine *memtx = va_arg(va, struct memtx_engine *);
 	while (!fiber_is_cancelled()) {
 		bool stop;
+		struct errinj *delay = errinj(ERRINJ_MEMTX_DELAY_GC,
+					      ERRINJ_BOOL);
+		if (delay != NULL && delay->bparam) {
+			while (delay->bparam)
+				fiber_sleep(0.001);
+		}
 		memtx_engine_run_gc(memtx, &stop);
 		if (stop) {
 			fiber_yield_timeout(TIMEOUT_INFINITY);
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 4aad8da..cbc5091 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -993,7 +993,7 @@ memtx_space_new(struct memtx_engine *memtx,
 		tuple_format_new(&memtx_tuple_format_vtab, memtx, keys, key_count,
 				 def->fields, def->field_count,
 				 def->exact_field_count, def->dict,
-				 def->opts.is_temporary);
+				 def->opts.is_temporary, def->opts.is_temporary);
 	if (format == NULL) {
 		free(memtx_space);
 		return NULL;
diff --git a/src/box/space.c b/src/box/space.c
index 6245167..316b34b 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -195,6 +195,7 @@ struct space *
 space_new_ephemeral(struct space_def *def, struct rlist *key_list)
 {
 	assert(def->opts.is_temporary);
+	assert(def->opts.is_ephemeral);
 	struct space *space = space_new(def, key_list);
 	if (space == NULL)
 		return NULL;
diff --git a/src/box/space_def.c b/src/box/space_def.c
index d60c2d3..c5b5ec2 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -53,6 +53,7 @@ checks_array_decode(const char **str, uint32_t len, char *opt, uint32_t errcode,
 const struct space_opts space_opts_default = {
 	/* .group_id = */ 0,
 	/* .is_temporary = */ false,
+	/* .is_ephemeral = */ false,
 	/* .view = */ false,
 	/* .sql        = */ NULL,
 	/* .checks     = */ NULL,
@@ -262,6 +263,7 @@ space_def_new_ephemeral(uint32_t field_count)
 {
 	struct space_opts opts = space_opts_default;
 	opts.is_temporary = true;
+	opts.is_ephemeral = true;
 	struct space_def *space_def = space_def_new(0, 0, field_count,
 						    "ephemeral",
 						    strlen("ephemeral"),
diff --git a/src/box/space_def.h b/src/box/space_def.h
index b6ab6b3..52ff567 100644
--- a/src/box/space_def.h
+++ b/src/box/space_def.h
@@ -58,6 +58,11 @@ struct space_opts {
          *   does not require manual release.
 	 */
 	bool is_temporary;
+	/**
+	 * This flag is set if space is ephemeral and hence
+	 * its format might be re-used.
+	 */
+	bool is_ephemeral;
 	/**
 	 * If the space is a view, then it can't feature any
 	 * indexes, and must have SQL statement. Moreover,
diff --git a/src/box/tuple.c b/src/box/tuple.c
index a87b2bd..9cdb464 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -203,12 +203,16 @@ tuple_next(struct tuple_iterator *it)
 int
 tuple_init(field_name_hash_f hash)
 {
+	if (tuple_format_init() != 0)
+		return -1;
+
 	field_name_hash = hash;
 	/*
 	 * Create a format for runtime tuples
 	 */
 	tuple_format_runtime = tuple_format_new(&tuple_format_runtime_vtab, NULL,
-						NULL, 0, NULL, 0, 0, NULL, false);
+						NULL, 0, NULL, 0, 0, NULL, false,
+						false);
 	if (tuple_format_runtime == NULL)
 		return -1;
 
@@ -380,7 +384,8 @@ box_tuple_format_new(struct key_def **keys, uint16_t key_count)
 {
 	box_tuple_format_t *format =
 		tuple_format_new(&tuple_format_runtime_vtab, NULL,
-				 keys, key_count, NULL, 0, 0, NULL, false);
+				 keys, key_count, NULL, 0, 0, NULL, false,
+				 false);
 	if (format != NULL)
 		tuple_format_ref(format);
 	return format;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 559c601..1e8675c 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -34,6 +34,8 @@
 #include "tuple_format.h"
 #include "coll_id_cache.h"
 
+#include "third_party/PMurHash.h"
+
 /** Global table of tuple formats */
 struct tuple_format **tuple_formats;
 static intptr_t recycled_format_ids = FORMAT_ID_NIL;
@@ -276,7 +278,12 @@ tuple_format_register(struct tuple_format *format)
 			formats_capacity = new_capacity;
 			tuple_formats = formats;
 		}
-		if (formats_size == FORMAT_ID_MAX + 1) {
+		uint32_t formats_size_max = FORMAT_ID_MAX + 1;
+		struct errinj *inj = errinj(ERRINJ_TUPLE_FORMAT_COUNT,
+					    ERRINJ_INT);
+		if (inj != NULL && inj->iparam > 0)
+			formats_size_max = inj->iparam;
+		if (formats_size >= formats_size_max) {
 			diag_set(ClientError, ER_TUPLE_FORMAT_LIMIT,
 				 (unsigned) formats_capacity);
 			return -1;
@@ -380,6 +387,69 @@ error:
 	return NULL;
 }
 
+static int
+tuple_format_cmp(const struct tuple_format *a, const struct tuple_format *b)
+{
+	if (a->exact_field_count != b->exact_field_count)
+		return a->exact_field_count - b->exact_field_count;
+	if (tuple_format_field_count(a) != tuple_format_field_count(b))
+		return tuple_format_field_count(a) - tuple_format_field_count(b);
+
+	for (uint32_t i = 0; i < tuple_format_field_count((struct tuple_format *)a); ++i) {
+		struct tuple_field *field_a = tuple_format_field(
+			(struct tuple_format *)a, i);
+		struct tuple_field *field_b = tuple_format_field(
+			(struct tuple_format *)b, i);
+		if (field_a->type != field_b->type)
+			return (int)field_a->type - (int)field_b->type;
+		if (field_a->coll_id != field_b->coll_id)
+			return (int)field_a->coll_id - (int)field_b->coll_id;
+		if (field_a->nullable_action != field_b->nullable_action)
+			return (int)field_a->nullable_action -
+				(int)field_b->nullable_action;
+		if (field_a->is_key_part != field_b->is_key_part)
+			return (int)field_a->is_key_part -
+				(int)field_b->is_key_part;
+	}
+
+	return 0;
+}
+
+static uint32_t
+tuple_format_hash(struct tuple_format *format)
+{
+#define TUPLE_FIELD_MEMBER_HASH(field, member, h, carry, size) \
+	PMurHash32_Process(&h, &carry, &field->member, \
+			   sizeof(field->member)); \
+	size += sizeof(field->member);
+
+	uint32_t h = 13;
+	uint32_t carry = 0;
+	uint32_t size = 0;
+	for (uint32_t i = 0; i < tuple_format_field_count(format); ++i) {
+		struct tuple_field *f = tuple_format_field(format, i);
+		TUPLE_FIELD_MEMBER_HASH(f, type, h, carry, size)
+		TUPLE_FIELD_MEMBER_HASH(f, coll_id, h, carry, size)
+		TUPLE_FIELD_MEMBER_HASH(f, nullable_action, h, carry, size)
+		TUPLE_FIELD_MEMBER_HASH(f, is_key_part, h, carry, size)
+	}
+#undef TUPLE_FIELD_MEMBER_HASH
+	return PMurHash32_Result(h, carry, size);
+}
+
+#define MH_SOURCE 1
+#define mh_name _tuple_format
+#define mh_key_t struct tuple_format *
+#define mh_node_t struct tuple_format *
+#define mh_arg_t void *
+#define mh_hash(a, arg) ((*(a))->hash)
+#define mh_hash_key(a, arg) ((a)->hash)
+#define mh_cmp(a, b, arg) (tuple_format_cmp(*(a), *(b)))
+#define mh_cmp_key(a, b, arg) (tuple_format_cmp((a), *(b)))
+#include "salad/mhash.h"
+
+struct mh_tuple_format_t *tuple_formats_hash = NULL;
+
 /** Free tuple format resources, doesn't unregister. */
 static inline void
 tuple_format_destroy(struct tuple_format *format)
@@ -392,17 +462,82 @@ tuple_format_destroy(struct tuple_format *format)
 void
 tuple_format_delete(struct tuple_format *format)
 {
+	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, NULL);
+	if (key != mh_end(tuple_formats_hash))
+		mh_tuple_format_del(tuple_formats_hash, key, NULL);
 	tuple_format_deregister(format);
 	tuple_format_destroy(format);
 	free(format);
 }
 
+/**
+ * Try to reuse given format. This is only possible for formats
+ * of ephemeral spaces, since we need to be sure that shared
+ * dictionary will never be altered. If it can, then alter can
+ * affect another space, which shares a format with one which is
+ * altered.
+ * @param p_format Double pointer to format. It is updated with
+ * 		   hashed value, if corresponding format was found
+ * 		   in hash table
+ * @retval Returns true if format was found in hash table, false
+ *	   otherwise.
+ *
+ */
+static bool
+tuple_format_reuse(struct tuple_format **p_format)
+{
+	struct tuple_format *format = *p_format;
+	if (!format->is_ephemeral)
+		return false;
+	/*
+	 * These fields do not participate in hashing.
+	 * Make sure they're unset.
+	 */
+	assert(format->dict->name_count == 0);
+	assert(format->is_temporary);
+	format->hash = tuple_format_hash(format);
+	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format,
+					    NULL);
+	if (key != mh_end(tuple_formats_hash)) {
+		struct tuple_format **entry = mh_tuple_format_node(
+			tuple_formats_hash, key);
+		tuple_format_destroy(format);
+		free(format);
+		*p_format = *entry;
+		return true;
+	}
+	return false;
+}
+
+/**
+ * See justification, why ephemeral space's formats are
+ * only feasible for hasing.
+ * @retval 0 on success, even if format wasn't added to hash
+ * 	   -1 in case of error.
+ */
+static int
+tuple_format_add_to_hash(struct tuple_format *format)
+{
+	if(!format->is_ephemeral)
+		return 0;
+	assert(format->dict->name_count == 0);
+	assert(format->is_temporary);
+	mh_int_t key = mh_tuple_format_put(tuple_formats_hash,
+					   (const struct tuple_format **)&format,
+					   NULL, NULL);
+	if (key == mh_end(tuple_formats_hash))
+		return -1;
+	else
+		return 0;
+}
+
 struct tuple_format *
 tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
 		 struct key_def * const *keys, uint16_t key_count,
 		 const struct field_def *space_fields,
 		 uint32_t space_field_count, uint32_t exact_field_count,
-		 struct tuple_dictionary *dict, bool is_temporary)
+		 struct tuple_dictionary *dict, bool is_temporary,
+		 bool is_ephemeral)
 {
 	struct tuple_format *format =
 		tuple_format_alloc(keys, key_count, space_field_count, dict);
@@ -411,18 +546,24 @@ tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
 	format->vtab = *vtab;
 	format->engine = engine;
 	format->is_temporary = is_temporary;
+	format->is_ephemeral = is_ephemeral;
 	format->exact_field_count = exact_field_count;
-	if (tuple_format_register(format) < 0) {
-		tuple_format_destroy(format);
-		free(format);
-		return NULL;
-	}
 	if (tuple_format_create(format, keys, key_count, space_fields,
-				space_field_count) < 0) {
-		tuple_format_delete(format);
-		return NULL;
+				space_field_count) < 0)
+		goto err;
+	if (tuple_format_reuse(&format))
+		return format;
+	if (tuple_format_register(format) < 0)
+		goto err;
+	if (tuple_format_add_to_hash(format) < 0) {
+		tuple_format_deregister(format);
+		goto err;
 	}
 	return format;
+err:
+	tuple_format_destroy(format);
+	free(format);
+	return NULL;
 }
 
 bool
@@ -823,3 +964,15 @@ error:
 		 tt_sprintf("error in path on position %d", rc));
 	return -1;
 }
+
+int
+tuple_format_init()
+{
+	tuple_formats_hash = mh_tuple_format_new();
+	if (tuple_formats_hash == NULL) {
+		diag_set(OutOfMemory, sizeof(struct mh_tuple_format_t), "malloc",
+			 "tuple format hash");
+		return -1;
+	}
+	return 0;
+}
diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
index d15fef2..3fc3a23 100644
--- a/src/box/tuple_format.h
+++ b/src/box/tuple_format.h
@@ -143,6 +143,10 @@ struct tuple_format {
 	void *engine;
 	/** Identifier */
 	uint16_t id;
+	/**
+	 * Hash key for shared formats of epeheral spaces.
+	 */
+	uint32_t hash;
 	/** Reference counter */
 	int refs;
 	/**
@@ -151,6 +155,11 @@ struct tuple_format {
 	 * in progress.
 	 */
 	bool is_temporary;
+	/**
+	 * This format belongs to ephemeral space and thus might
+	 * be shared with other ephemeral spaces.
+	 */
+	bool is_ephemeral;
 	/**
 	 * Size of field map of tuple in bytes.
 	 * \sa struct tuple
@@ -200,7 +209,7 @@ struct tuple_format {
  * a given format.
  */
 static inline uint32_t
-tuple_format_field_count(struct tuple_format *format)
+tuple_format_field_count(const struct tuple_format *format)
 {
 	const struct json_token *root = &format->fields.root;
 	return root->children != NULL ? root->max_child_idx + 1 : 0;
@@ -265,6 +274,7 @@ tuple_format_unref(struct tuple_format *format)
  * @param space_field_count Length of @a space_fields.
  * @param exact_field_count Exact field count for format.
  * @param is_temporary Set if format belongs to temporary space.
+ * @param is_ephemeral Set if format belongs to ephemeral space.
  *
  * @retval not NULL Tuple format.
  * @retval     NULL Memory error.
@@ -274,7 +284,8 @@ tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
 		 struct key_def * const *keys, uint16_t key_count,
 		 const struct field_def *space_fields,
 		 uint32_t space_field_count, uint32_t exact_field_count,
-		 struct tuple_dictionary *dict, bool is_temporary);
+		 struct tuple_dictionary *dict, bool is_temporary,
+		 bool is_ephemeral);
 
 /**
  * Check, if @a format1 can store any tuples of @a format2. For
@@ -459,6 +470,13 @@ tuple_field_by_part_raw(struct tuple_format *format, const char *data,
 	return tuple_field_raw(format, data, field_map, part->fieldno);
 }
 
+/**
+ * Initialize tuple format subsystem.
+ * @retval 0 on success, -1 otherwise.
+ */
+int
+tuple_format_init();
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 49e8372..b66360d 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -621,7 +621,8 @@ vinyl_engine_create_space(struct engine *engine, struct space_def *def,
 	struct tuple_format *format =
 		tuple_format_new(&vy_tuple_format_vtab, NULL, keys, key_count,
 				 def->fields, def->field_count,
-				 def->exact_field_count, def->dict, false);
+				 def->exact_field_count, def->dict, false,
+				 false);
 	if (format == NULL) {
 		free(space);
 		return NULL;
@@ -3045,7 +3046,7 @@ vy_send_lsm(struct vy_join_ctx *ctx, struct vy_lsm_recovery_info *lsm_info)
 		goto out;
 	ctx->format = tuple_format_new(&vy_tuple_format_vtab, NULL,
 				       &ctx->key_def, 1, NULL, 0, 0, NULL,
-				       false);
+				       false, false);
 	if (ctx->format == NULL)
 		goto out_free_key_def;
 	tuple_format_ref(ctx->format);
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 7a6ba6e..570e783 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -61,7 +61,8 @@ vy_lsm_env_create(struct vy_lsm_env *env, const char *path,
 		  void *upsert_thresh_arg)
 {
 	env->key_format = tuple_format_new(&vy_tuple_format_vtab, NULL,
-					   NULL, 0, NULL, 0, 0, NULL, false);
+					   NULL, 0, NULL, 0, 0, NULL, false,
+					   false);
 	if (env->key_format == NULL)
 		return -1;
 	tuple_format_ref(env->key_format);
@@ -155,7 +156,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 	} else {
 		lsm->disk_format = tuple_format_new(&vy_tuple_format_vtab, NULL,
 						    &cmp_def, 1, NULL, 0, 0,
-						    NULL, false);
+						    NULL, false, false);
 		if (lsm->disk_format == NULL)
 			goto fail_format;
 	}
diff --git a/src/errinj.h b/src/errinj.h
index 39de63d..31e4dfb 100644
--- a/src/errinj.h
+++ b/src/errinj.h
@@ -123,6 +123,8 @@ struct errinj {
 	_(ERRINJ_RELAY_BREAK_LSN, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_WAL_BREAK_LSN, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_VY_COMPACTION_DELAY, ERRINJ_BOOL, {.bparam = false}) \
+	_(ERRINJ_TUPLE_FORMAT_COUNT, ERRINJ_INT, {.iparam = -1}) \
+	_(ERRINJ_MEMTX_DELAY_GC, ERRINJ_BOOL, {.bparam = false}) \
 
 ENUM0(errinj_id, ERRINJ_LIST);
 extern struct errinj errinjs[];
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 1230367..ccb9aff 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -46,8 +46,12 @@ errinj.info()
     state: false
   ERRINJ_WAL_FALLOCATE:
     state: 0
+  ERRINJ_RELAY_EXIT_DELAY:
+    state: 0
   ERRINJ_SNAP_COMMIT_DELAY:
     state: false
+  ERRINJ_TUPLE_FORMAT_COUNT:
+    state: -1
   ERRINJ_TUPLE_ALLOC:
     state: false
   ERRINJ_VY_RUN_WRITE_DELAY:
@@ -92,8 +96,8 @@ errinj.info()
     state: false
   ERRINJ_VY_POINT_ITER_WAIT:
     state: false
-  ERRINJ_RELAY_EXIT_DELAY:
-    state: 0
+  ERRINJ_MEMTX_DELAY_GC:
+    state: false
   ERRINJ_IPROTO_TX_DELAY:
     state: false
   ERRINJ_BUILD_INDEX:
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index cb993f8..c423c8b 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -16,6 +16,42 @@ errinj = box.error.injection
 fiber = require('fiber')
 ---
 ...
+-- gh-3924 Check that tuple_formats of ephemeral spaces are
+-- reused.
+box.sql.execute("CREATE TABLE t4 (id INTEGER PRIMARY KEY, a INTEGER);")
+---
+...
+box.sql.execute("INSERT INTO t4 VALUES (1,1)")
+---
+...
+box.sql.execute("INSERT INTO t4 VALUES (2,1)")
+---
+...
+box.sql.execute("INSERT INTO t4 VALUES (3,2)")
+---
+...
+errinj.set('ERRINJ_TUPLE_FORMAT_COUNT', 200)
+---
+- ok
+...
+errinj.set('ERRINJ_MEMTX_DELAY_GC', true)
+---
+- ok
+...
+for i = 1, 201 do box.sql.execute("SELECT DISTINCT a FROM t4") end
+---
+...
+errinj.set('ERRINJ_MEMTX_DELAY_GC', false)
+---
+- ok
+...
+errinj.set('ERRINJ_TUPLE_FORMAT_COUNT', -1)
+---
+- ok
+...
+box.sql.execute('DROP TABLE t4')
+---
+...
 box.sql.execute('create table test (id int primary key, a float, b text)')
 ---
 ...
diff --git a/test/sql/errinj.test.lua b/test/sql/errinj.test.lua
index fa7f9f2..8378c25 100644
--- a/test/sql/errinj.test.lua
+++ b/test/sql/errinj.test.lua
@@ -5,6 +5,19 @@ box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
 errinj = box.error.injection
 fiber = require('fiber')
 
+-- gh-3924 Check that tuple_formats of ephemeral spaces are
+-- reused.
+box.sql.execute("CREATE TABLE t4 (id INTEGER PRIMARY KEY, a INTEGER);")
+box.sql.execute("INSERT INTO t4 VALUES (1,1)")
+box.sql.execute("INSERT INTO t4 VALUES (2,1)")
+box.sql.execute("INSERT INTO t4 VALUES (3,2)")
+errinj.set('ERRINJ_TUPLE_FORMAT_COUNT', 200)
+errinj.set('ERRINJ_MEMTX_DELAY_GC', true)
+for i = 1, 201 do box.sql.execute("SELECT DISTINCT a FROM t4") end
+errinj.set('ERRINJ_MEMTX_DELAY_GC', false)
+errinj.set('ERRINJ_TUPLE_FORMAT_COUNT', -1)
+box.sql.execute('DROP TABLE t4')
+
 box.sql.execute('create table test (id int primary key, a float, b text)')
 box.schema.user.grant('guest','read,write,execute', 'universe')
 cn = remote.connect(box.cfg.listen)
diff --git a/test/unit/tuple_bigref.c b/test/unit/tuple_bigref.c
index 20eab61..76a2e98 100644
--- a/test/unit/tuple_bigref.c
+++ b/test/unit/tuple_bigref.c
@@ -143,6 +143,7 @@ main()
 
 	memory_init();
 	fiber_init(fiber_c_invoke);
+	tuple_format_init();
 	tuple_init(NULL);
 
 	tuple_end = mp_encode_array(tuple_end, 1);
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index 6808ff4..395ad15 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -17,12 +17,13 @@ vy_iterator_C_test_init(size_t cache_size)
 	say_set_log_level(S_WARN);
 
 	memory_init();
+	tuple_format_init();
 	fiber_init(fiber_c_invoke);
 	tuple_init(NULL);
 	vy_cache_env_create(&cache_env, cord_slab_cache());
 	vy_cache_env_set_quota(&cache_env, cache_size);
 	vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, NULL, 0,
-					 NULL, 0, 0, NULL, false);
+					 NULL, 0, 0, NULL, false, false);
 	tuple_format_ref(vy_key_format);
 
 	size_t mem_size = 64 * 1024 * 1024;
@@ -202,7 +203,8 @@ create_test_mem(struct key_def *def)
 	struct key_def * const defs[] = { def };
 	struct tuple_format *format =
 		tuple_format_new(&vy_tuple_format_vtab, NULL, defs,
-				 def->part_count, NULL, 0, 0, NULL, false);
+				 def->part_count, NULL, 0, 0, NULL, false,
+				 false);
 	fail_if(format == NULL);
 
 	/* Create mem */
@@ -220,7 +222,7 @@ create_test_cache(uint32_t *fields, uint32_t *types,
 	assert(*def != NULL);
 	vy_cache_create(cache, &cache_env, *def, true);
 	*format = tuple_format_new(&vy_tuple_format_vtab, NULL, def, 1, NULL, 0,
-				   0, NULL, false);
+				   0, NULL, false, false);
 	tuple_format_ref(*format);
 }
 
diff --git a/test/unit/vy_mem.c b/test/unit/vy_mem.c
index a13c58b..b8272ea 100644
--- a/test/unit/vy_mem.c
+++ b/test/unit/vy_mem.c
@@ -78,7 +78,8 @@ test_iterator_restore_after_insertion()
 	/* Create format */
 	struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
 						       NULL, &key_def, 1, NULL,
-						       0, 0, NULL, false);
+						       0, 0, NULL, false,
+						       false);
 	assert(format != NULL);
 	tuple_format_ref(format);
 
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index c19d307..8ed16f6 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -85,7 +85,8 @@ test_basic()
 	vy_cache_create(&cache, &cache_env, key_def, true);
 	struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
 						       NULL, &key_def, 1, NULL,
-						       0, 0, NULL, false);
+						       0, 0, NULL, false,
+						       false);
 	isnt(format, NULL, "tuple_format_new is not NULL");
 	tuple_format_ref(format);
 
-- 
2.19.1

Re: [PATCH v2 1/4] Pass necessary fields to tuple_format contructor

[Vladimir Davydov]

On Thu, Jan 24, 2019 at 03:48:45PM +0300, Kirill Yukhin wrote:
> There were three extra fields of tuple_format which were setup
> after it was created. Fix that by extending tuple_format
> contstructor w/ three new arguments: engine, is_temporary,
> exact_field_count.
> ---
>  src/box/blackhole.c             |  6 +++---
>  src/box/memtx_space.c           |  9 ++++-----
>  src/box/tuple.c                 |  8 ++++----
>  src/box/tuple_format.c          | 13 ++++++++-----
>  src/box/tuple_format.h          | 11 ++++++++---
>  src/box/vinyl.c                 | 11 ++++++-----
>  src/box/vy_lsm.c                |  9 +++++----
>  test/unit/vy_iterators_helper.c | 12 ++++++------
>  test/unit/vy_mem.c              |  4 ++--
>  test/unit/vy_point_lookup.c     |  4 ++--
>  10 files changed, 48 insertions(+), 39 deletions(-)

Pushed to 2.1

Re: [PATCH v2 2/4] Set is_temporary flag for formats of ephemeral spaces

[Vladimir Davydov]

On Thu, Jan 24, 2019 at 03:48:46PM +0300, Kirill Yukhin wrote:
> Before the patch, when ephemeral space was created flag
> is_temporary was set after space was actually created.
> Which in turn lead to corresponding flag of tuple_format
> being set to `false`.
> So, having heavy load using ephemeral spaces (almost any
> SQL query) and snapshotting at the same time might lead
> to OOM, since tuples of ephemeral spaces were not marked
> as temporary and were not gc-ed.
> Patch sets the flag in space definition.
> ---
>  src/box/space.c     |  2 +-
>  src/box/space_def.c | 14 ++++++++++++++
>  src/box/space_def.h |  9 +++++++++
>  src/box/sql.c       |  6 +-----
>  4 files changed, 25 insertions(+), 6 deletions(-)

Pushed to 2.1

Re: [PATCH v2 3/4] sql: set error type in case of ephemral space creation failure

[Vladimir Davydov]

On Thu, Jan 24, 2019 at 03:48:47PM +0300, Kirill Yukhin wrote:
> This is trivial patch which sets error kind if epehemeral
> spaces cannot be created due to Tarantool's backend (e.g. there's
> no more memory or formats available).
> ---
>  src/box/sql/vdbe.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Pushed to 2.1

Re: [PATCH v2 4/4] Allow to reuse tuple_formats for ephemeral spaces

[Vladimir Davydov]

On Thu, Jan 24, 2019 at 03:48:48PM +0300, Kirill Yukhin wrote:
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 4aad8da..cbc5091 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -993,7 +993,7 @@ memtx_space_new(struct memtx_engine *memtx,
>  		tuple_format_new(&memtx_tuple_format_vtab, memtx, keys, key_count,
>  				 def->fields, def->field_count,
>  				 def->exact_field_count, def->dict,
> -				 def->opts.is_temporary);
> +				 def->opts.is_temporary, def->opts.is_temporary);

is_temporary, is_temporary WTF?

>  	if (format == NULL) {
>  		free(memtx_space);
>  		return NULL;
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 559c601..1e8675c 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -380,6 +387,69 @@ error:
>  	return NULL;
>  }
>  
> +static int
> +tuple_format_cmp(const struct tuple_format *a, const struct tuple_format *b)
> +{
> +	if (a->exact_field_count != b->exact_field_count)
> +		return a->exact_field_count - b->exact_field_count;
> +	if (tuple_format_field_count(a) != tuple_format_field_count(b))
> +		return tuple_format_field_count(a) - tuple_format_field_count(b);
> +
> +	for (uint32_t i = 0; i < tuple_format_field_count((struct tuple_format *)a); ++i) {
                                                          ^^^^^^^^^^^^^^^^^^^^^^^^
You forgot to drop this type conversion.

> +		struct tuple_field *field_a = tuple_format_field(
> +			(struct tuple_format *)a, i);
> +		struct tuple_field *field_b = tuple_format_field(
> +			(struct tuple_format *)b, i);
> +		if (field_a->type != field_b->type)
> +			return (int)field_a->type - (int)field_b->type;
> +		if (field_a->coll_id != field_b->coll_id)
> +			return (int)field_a->coll_id - (int)field_b->coll_id;
> +		if (field_a->nullable_action != field_b->nullable_action)
> +			return (int)field_a->nullable_action -
> +				(int)field_b->nullable_action;
> +		if (field_a->is_key_part != field_b->is_key_part)
> +			return (int)field_a->is_key_part -
> +				(int)field_b->is_key_part;
> +	}
> +
> +	return 0;
> +}
> +
> +static uint32_t
> +tuple_format_hash(struct tuple_format *format)
> +{
> +#define TUPLE_FIELD_MEMBER_HASH(field, member, h, carry, size) \
> +	PMurHash32_Process(&h, &carry, &field->member, \
> +			   sizeof(field->member)); \
> +	size += sizeof(field->member);
> +
> +	uint32_t h = 13;
> +	uint32_t carry = 0;
> +	uint32_t size = 0;
> +	for (uint32_t i = 0; i < tuple_format_field_count(format); ++i) {
> +		struct tuple_field *f = tuple_format_field(format, i);
> +		TUPLE_FIELD_MEMBER_HASH(f, type, h, carry, size)
> +		TUPLE_FIELD_MEMBER_HASH(f, coll_id, h, carry, size)
> +		TUPLE_FIELD_MEMBER_HASH(f, nullable_action, h, carry, size)
> +		TUPLE_FIELD_MEMBER_HASH(f, is_key_part, h, carry, size)
> +	}
> +#undef TUPLE_FIELD_MEMBER_HASH
> +	return PMurHash32_Result(h, carry, size);
> +}
> +
> +#define MH_SOURCE 1
> +#define mh_name _tuple_format
> +#define mh_key_t struct tuple_format *
> +#define mh_node_t struct tuple_format *
> +#define mh_arg_t void *
> +#define mh_hash(a, arg) ((*(a))->hash)
> +#define mh_hash_key(a, arg) ((a)->hash)
> +#define mh_cmp(a, b, arg) (tuple_format_cmp(*(a), *(b)))
> +#define mh_cmp_key(a, b, arg) (tuple_format_cmp((a), *(b)))
> +#include "salad/mhash.h"
> +
> +struct mh_tuple_format_t *tuple_formats_hash = NULL;
> +
>  /** Free tuple format resources, doesn't unregister. */
>  static inline void
>  tuple_format_destroy(struct tuple_format *format)
> @@ -392,17 +462,82 @@ tuple_format_destroy(struct tuple_format *format)
>  void
>  tuple_format_delete(struct tuple_format *format)
>  {
> +	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, NULL);
> +	if (key != mh_end(tuple_formats_hash))
> +		mh_tuple_format_del(tuple_formats_hash, key, NULL);

Please factor this out to a separate function complimentary to
tuple_format_add_to_hash (tuple_format_remove_from_hash?).

>  	tuple_format_deregister(format);
>  	tuple_format_destroy(format);
>  	free(format);
>  }
>  
> +/**
> + * Try to reuse given format. This is only possible for formats
> + * of ephemeral spaces, since we need to be sure that shared
> + * dictionary will never be altered. If it can, then alter can
> + * affect another space, which shares a format with one which is
> + * altered.
> + * @param p_format Double pointer to format. It is updated with
> + * 		   hashed value, if corresponding format was found
> + * 		   in hash table
> + * @retval Returns true if format was found in hash table, false
> + *	   otherwise.
> + *
> + */
> +static bool
> +tuple_format_reuse(struct tuple_format **p_format)
> +{
> +	struct tuple_format *format = *p_format;
> +	if (!format->is_ephemeral)
> +		return false;
> +	/*
> +	 * These fields do not participate in hashing.
> +	 * Make sure they're unset.
> +	 */
> +	assert(format->dict->name_count == 0);
> +	assert(format->is_temporary);
> +	format->hash = tuple_format_hash(format);

The fact that in tuple_format_add_to_hash you implicitly rely on
format->hash having been initialized here looks confusing. Let's
initialize hash for all formats.

> +	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format,
> +					    NULL);
> +	if (key != mh_end(tuple_formats_hash)) {
> +		struct tuple_format **entry = mh_tuple_format_node(
> +			tuple_formats_hash, key);
> +		tuple_format_destroy(format);
> +		free(format);
> +		*p_format = *entry;
> +		return true;
> +	}
> +	return false;
> +}
> +
> +/**
> + * See justification, why ephemeral space's formats are
> + * only feasible for hasing.
> + * @retval 0 on success, even if format wasn't added to hash
> + * 	   -1 in case of error.
> + */
> +static int
> +tuple_format_add_to_hash(struct tuple_format *format)
> +{
> +	if(!format->is_ephemeral)
> +		return 0;
> +	assert(format->dict->name_count == 0);
> +	assert(format->is_temporary);
> +	mh_int_t key = mh_tuple_format_put(tuple_formats_hash,
> +					   (const struct tuple_format **)&format,
> +					   NULL, NULL);
> +	if (key == mh_end(tuple_formats_hash))

You forgot to set diag (say hi to #3534 again ;-)

> +		return -1;
> +	else
> +		return 0;
> +}
> +
>  struct tuple_format *
>  tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
>  		 struct key_def * const *keys, uint16_t key_count,
>  		 const struct field_def *space_fields,
>  		 uint32_t space_field_count, uint32_t exact_field_count,
> -		 struct tuple_dictionary *dict, bool is_temporary)
> +		 struct tuple_dictionary *dict, bool is_temporary,
> +		 bool is_ephemeral)
>  {
>  	struct tuple_format *format =
>  		tuple_format_alloc(keys, key_count, space_field_count, dict);
> diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
> index d15fef2..3fc3a23 100644
> --- a/src/box/tuple_format.h
> +++ b/src/box/tuple_format.h
> @@ -459,6 +470,13 @@ tuple_field_by_part_raw(struct tuple_format *format, const char *data,
>  	return tuple_field_raw(format, data, field_map, part->fieldno);
>  }
>  
> +/**
> + * Initialize tuple format subsystem.
> + * @retval 0 on success, -1 otherwise.
> + */
> +int
> +tuple_format_init();
> +

tuple_format_init is declared in the very end of the file while
tuple_format_free in the very beginning. Urghhh. Please fix.

>  #if defined(__cplusplus)
>  } /* extern "C" */
>  #endif /* defined(__cplusplus) */
> diff --git a/test/unit/tuple_bigref.c b/test/unit/tuple_bigref.c
> index 20eab61..76a2e98 100644
> --- a/test/unit/tuple_bigref.c
> +++ b/test/unit/tuple_bigref.c
> @@ -143,6 +143,7 @@ main()
>  
>  	memory_init();
>  	fiber_init(fiber_c_invoke);
> +	tuple_format_init();

This is redundant. Remove please.

>  	tuple_init(NULL);
>  
>  	tuple_end = mp_encode_array(tuple_end, 1);
> diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
> index 6808ff4..395ad15 100644
> --- a/test/unit/vy_iterators_helper.c
> +++ b/test/unit/vy_iterators_helper.c
> @@ -17,12 +17,13 @@ vy_iterator_C_test_init(size_t cache_size)
>  	say_set_log_level(S_WARN);
>  
>  	memory_init();
> +	tuple_format_init();

Ditto.

>  	fiber_init(fiber_c_invoke);
>  	tuple_init(NULL);
>  	vy_cache_env_create(&cache_env, cord_slab_cache());
>  	vy_cache_env_set_quota(&cache_env, cache_size);
>  	vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, NULL, 0,
> -					 NULL, 0, 0, NULL, false);
> +					 NULL, 0, 0, NULL, false, false);
>  	tuple_format_ref(vy_key_format);
>  
>  	size_t mem_size = 64 * 1024 * 1024;

Re: [PATCH v2 4/4] Allow to reuse tuple_formats for ephemeral spaces

[Kirill Yukhin]

Hello Vladimir,

Thanks for review!
My answers are inlined, iterative patch is in the bottom.

--
Regards, Kirill Yukhin
On 24 Jan 20:49, Vladimir Davydov wrote:
> On Thu, Jan 24, 2019 at 03:48:48PM +0300, Kirill Yukhin wrote:
> > diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> > index 4aad8da..cbc5091 100644
> > --- a/src/box/memtx_space.c
> > +++ b/src/box/memtx_space.c
> > @@ -993,7 +993,7 @@ memtx_space_new(struct memtx_engine *memtx,
> >  		tuple_format_new(&memtx_tuple_format_vtab, memtx, keys, key_count,
> >  				 def->fields, def->field_count,
> >  				 def->exact_field_count, def->dict,
> > -				 def->opts.is_temporary);
> > +				 def->opts.is_temporary, def->opts.is_temporary);
> 
> is_temporary, is_temporary WTF?

Fixed.

> > diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> > index 559c601..1e8675c 100644
> > --- a/src/box/tuple_format.c
> > +++ b/src/box/tuple_format.c
> > @@ -380,6 +387,69 @@ error:
> >  	return NULL;
> >  }
> >  
> > +static int
> > +tuple_format_cmp(const struct tuple_format *a, const struct tuple_format *b)
> > +{
> > +	if (a->exact_field_count != b->exact_field_count)
> > +		return a->exact_field_count - b->exact_field_count;
> > +	if (tuple_format_field_count(a) != tuple_format_field_count(b))
> > +		return tuple_format_field_count(a) - tuple_format_field_count(b);
> > +
> > +	for (uint32_t i = 0; i < tuple_format_field_count((struct tuple_format *)a); ++i) {
>                                                           ^^^^^^^^^^^^^^^^^^^^^^^^
> You forgot to drop this type conversion.

Removed.

> > +struct mh_tuple_format_t *tuple_formats_hash = NULL;
> > +
> >  /** Free tuple format resources, doesn't unregister. */
> >  static inline void
> >  tuple_format_destroy(struct tuple_format *format)
> > @@ -392,17 +462,82 @@ tuple_format_destroy(struct tuple_format *format)
> >  void
> >  tuple_format_delete(struct tuple_format *format)
> >  {
> > +	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, NULL);
> > +	if (key != mh_end(tuple_formats_hash))
> > +		mh_tuple_format_del(tuple_formats_hash, key, NULL);
> 
> Please factor this out to a separate function complimentary to
> tuple_format_add_to_hash (tuple_format_remove_from_hash?).

Done.
 
> > +static bool
> > +tuple_format_reuse(struct tuple_format **p_format)
> > +{
> > +	struct tuple_format *format = *p_format;
> > +	if (!format->is_ephemeral)
> > +		return false;
> > +	/*
> > +	 * These fields do not participate in hashing.
> > +	 * Make sure they're unset.
> > +	 */
> > +	assert(format->dict->name_count == 0);
> > +	assert(format->is_temporary);
> > +	format->hash = tuple_format_hash(format);
> 
> The fact that in tuple_format_add_to_hash you implicitly rely on
> format->hash having been initialized here looks confusing. Let's
> initialize hash for all formats.

I moved assignement to tuple_format_new().

> > +static int
> > +tuple_format_add_to_hash(struct tuple_format *format)
> > +{
> > +	if(!format->is_ephemeral)
> > +		return 0;
> > +	assert(format->dict->name_count == 0);
> > +	assert(format->is_temporary);
> > +	mh_int_t key = mh_tuple_format_put(tuple_formats_hash,
> > +					   (const struct tuple_format **)&format,
> > +					   NULL, NULL);
> > +	if (key == mh_end(tuple_formats_hash))
> 
> You forgot to set diag (say hi to #3534 again ;-)

Fixed.

> > +		return -1;
> > +	else
> > +		return 0;
> > +}
> > +
> >  struct tuple_format *
> >  tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
> >  		 struct key_def * const *keys, uint16_t key_count,
> >  		 const struct field_def *space_fields,
> >  		 uint32_t space_field_count, uint32_t exact_field_count,
> > -		 struct tuple_dictionary *dict, bool is_temporary)
> > +		 struct tuple_dictionary *dict, bool is_temporary,
> > +		 bool is_ephemeral)
> >  {
> >  	struct tuple_format *format =
> >  		tuple_format_alloc(keys, key_count, space_field_count, dict);
> > diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
> > index d15fef2..3fc3a23 100644
> > --- a/src/box/tuple_format.h
> > +++ b/src/box/tuple_format.h
> > @@ -459,6 +470,13 @@ tuple_field_by_part_raw(struct tuple_format *format, const char *data,
> >  	return tuple_field_raw(format, data, field_map, part->fieldno);
> >  }
> >  
> > +/**
> > + * Initialize tuple format subsystem.
> > + * @retval 0 on success, -1 otherwise.
> > + */
> > +int
> > +tuple_format_init();
> > +
> 
> tuple_format_init is declared in the very end of the file while
> tuple_format_free in the very beginning. Urghhh. Please fix.

I moved tuple format init before free.

> > diff --git a/test/unit/tuple_bigref.c b/test/unit/tuple_bigref.c
> > index 20eab61..76a2e98 100644
> > --- a/test/unit/tuple_bigref.c
> > +++ b/test/unit/tuple_bigref.c
> > @@ -143,6 +143,7 @@ main()
> >  
> >  	memory_init();
> >  	fiber_init(fiber_c_invoke);
> > +	tuple_format_init();
> 
> This is redundant. Remove please.

Removed.

> > diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
> > index 6808ff4..395ad15 100644
> > --- a/test/unit/vy_iterators_helper.c
> > +++ b/test/unit/vy_iterators_helper.c
> > @@ -17,12 +17,13 @@ vy_iterator_C_test_init(size_t cache_size)
> >  	say_set_log_level(S_WARN);
> >  
> >  	memory_init();
> > +	tuple_format_init();
> 
> Ditto.

Ditto.

--
Regards, Kirill Yukhin

diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index cbc5091..5f8cc98 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -993,7 +993,7 @@ memtx_space_new(struct memtx_engine *memtx,
 		tuple_format_new(&memtx_tuple_format_vtab, memtx, keys, key_count,
 				 def->fields, def->field_count,
 				 def->exact_field_count, def->dict,
-				 def->opts.is_temporary, def->opts.is_temporary);
+				 def->opts.is_temporary, def->opts.is_ephemeral);
 	if (format == NULL) {
 		free(memtx_space);
 		return NULL;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 1e8675c..305834e 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -395,7 +395,7 @@ tuple_format_cmp(const struct tuple_format *a, const struct tuple_format *b)
 	if (tuple_format_field_count(a) != tuple_format_field_count(b))
 		return tuple_format_field_count(a) - tuple_format_field_count(b);
 
-	for (uint32_t i = 0; i < tuple_format_field_count((struct tuple_format *)a); ++i) {
+	for (uint32_t i = 0; i < tuple_format_field_count(a); ++i) {
 		struct tuple_field *field_a = tuple_format_field(
 			(struct tuple_format *)a, i);
 		struct tuple_field *field_b = tuple_format_field(
@@ -459,12 +459,18 @@ tuple_format_destroy(struct tuple_format *format)
 	tuple_dictionary_unref(format->dict);
 }
 
-void
-tuple_format_delete(struct tuple_format *format)
+static void
+tuple_format_remove_from_hash(struct tuple_format *format)
 {
 	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, NULL);
 	if (key != mh_end(tuple_formats_hash))
 		mh_tuple_format_del(tuple_formats_hash, key, NULL);
+}
+
+void
+tuple_format_delete(struct tuple_format *format)
+{
+	tuple_format_remove_from_hash(format);
 	tuple_format_deregister(format);
 	tuple_format_destroy(format);
 	free(format);
@@ -495,7 +501,6 @@ tuple_format_reuse(struct tuple_format **p_format)
 	 */
 	assert(format->dict->name_count == 0);
 	assert(format->is_temporary);
-	format->hash = tuple_format_hash(format);
 	mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format,
 					    NULL);
 	if (key != mh_end(tuple_formats_hash)) {
@@ -525,10 +530,13 @@ tuple_format_add_to_hash(struct tuple_format *format)
 	mh_int_t key = mh_tuple_format_put(tuple_formats_hash,
 					   (const struct tuple_format **)&format,
 					   NULL, NULL);
-	if (key == mh_end(tuple_formats_hash))
+	if (key == mh_end(tuple_formats_hash)) {
+		diag_set(OutOfMemory, 0, "tuple_format_add_to_hash",
+			 "tuple formats hash entry");
 		return -1;
-	else
+	} else {
 		return 0;
+	}
 }
 
 struct tuple_format *
@@ -551,6 +559,7 @@ tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
 	if (tuple_format_create(format, keys, key_count, space_fields,
 				space_field_count) < 0)
 		goto err;
+	format->hash = tuple_format_hash(format);
 	if (tuple_format_reuse(&format))
 		return format;
 	if (tuple_format_register(format) < 0)
@@ -747,6 +756,18 @@ tuple_format_min_field_count(struct key_def * const *keys, uint16_t key_count,
 	return min_field_count;
 }
 
+int
+tuple_format_init()
+{
+	tuple_formats_hash = mh_tuple_format_new();
+	if (tuple_formats_hash == NULL) {
+		diag_set(OutOfMemory, sizeof(struct mh_tuple_format_t), "malloc",
+			 "tuple format hash");
+		return -1;
+	}
+	return 0;
+}
+
 /** Destroy tuple format subsystem and free resourses */
 void
 tuple_format_free()
@@ -964,15 +985,3 @@ error:
 		 tt_sprintf("error in path on position %d", rc));
 	return -1;
 }
-
-int
-tuple_format_init()
-{
-	tuple_formats_hash = mh_tuple_format_new();
-	if (tuple_formats_hash == NULL) {
-		diag_set(OutOfMemory, sizeof(struct mh_tuple_format_t), "malloc",
-			 "tuple format hash");
-		return -1;
-	}
-	return 0;
-}
diff --git a/test/unit/tuple_bigref.c b/test/unit/tuple_bigref.c
index 76a2e98..20eab61 100644
--- a/test/unit/tuple_bigref.c
+++ b/test/unit/tuple_bigref.c
@@ -143,7 +143,6 @@ main()
 
 	memory_init();
 	fiber_init(fiber_c_invoke);
-	tuple_format_init();
 	tuple_init(NULL);
 
 	tuple_end = mp_encode_array(tuple_end, 1);
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index 395ad15..55d8504 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -17,7 +17,6 @@ vy_iterator_C_test_init(size_t cache_size)
 	say_set_log_level(S_WARN);
 
 	memory_init();
-	tuple_format_init();
 	fiber_init(fiber_c_invoke);
 	tuple_init(NULL);
 	vy_cache_env_create(&cache_env, cord_slab_cache());

Re: [PATCH v2 4/4] Allow to reuse tuple_formats for ephemeral spaces

[Vladimir Davydov]

Pushed the patch to 2.1 with some minor changes:
 - moved tuple_format_remove_from_hash after tuple_format_add_to_hash;
 - moved tuple_format_cmp and tuple_format_hash to the beginning of
   the file;
 - removed the hunk making tuple_format_field take const format -
   added a cast removing const to tuple_format_cmp instead.