Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH 0/2] vinyl: zap upsert_format
@ 2018-03-30 15:47 Vladimir Davydov
  2018-03-30 15:47 ` [PATCH 1/2] vinyl: allocate upsert counter on lsregion Vladimir Davydov
  2018-03-30 15:47 ` [PATCH 2/2] vinyl: zap upsert_format Vladimir Davydov
  0 siblings, 2 replies; 7+ messages in thread
From: Vladimir Davydov @ 2018-03-30 15:47 UTC (permalink / raw)
  To: kostja; +Cc: v.shpilevoy, tarantool-patches

The sole purpose of a special format for UPSERT statements is that we
need to store UPSERT counter for them so that we can schedule UPSERT
squashing as soon as the number of UPSERTs for the same key exceeds a
certain threshold.

However, we only need to maintain the UPSERT counter for statements of
the memory level, so instead of maintaining yet another tuple format,
we can allocate the UPSERT counter on lsregion, near the tuple it is
for. This simplifies the code a great deal and eases implementation of
online ALTER.

https://github.com/tarantool/tarantool/tree/vy-zap-upsert-format

Vladimir Davydov (2):
  vinyl: allocate upsert counter on lsregion
  vinyl: zap upsert_format

 src/box/vinyl.c                 | 33 +++----------------
 src/box/vy_lsm.c                | 48 ++++++++-------------------
 src/box/vy_lsm.h                |  5 ---
 src/box/vy_mem.c                | 15 +++------
 src/box/vy_mem.h                |  6 +---
 src/box/vy_point_lookup.c       |  6 ++--
 src/box/vy_read_iterator.c      |  5 ++-
 src/box/vy_run.c                | 33 ++++++++-----------
 src/box/vy_run.h                | 11 ++-----
 src/box/vy_scheduler.c          |  8 ++---
 src/box/vy_stmt.c               | 72 ++++++++++++-----------------------------
 src/box/vy_stmt.h               | 49 +++++++++-------------------
 src/box/vy_tx.c                 |  9 ++----
 src/box/vy_upsert.c             | 11 +++----
 src/box/vy_upsert.h             |  3 +-
 src/box/vy_write_iterator.c     | 27 ++++++----------
 src/box/vy_write_iterator.h     |  8 ++---
 test/unit/vy_cache.c            | 11 +++----
 test/unit/vy_iterators_helper.c | 28 ++++++----------
 test/unit/vy_iterators_helper.h |  4 ---
 test/unit/vy_point_lookup.c     | 18 ++++-------
 test/unit/vy_write_iterator.c   |  6 ++--
 22 files changed, 125 insertions(+), 291 deletions(-)

-- 
2.11.0

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

* [PATCH 1/2] vinyl: allocate upsert counter on lsregion
  2018-03-30 15:47 [PATCH 0/2] vinyl: zap upsert_format Vladimir Davydov
@ 2018-03-30 15:47 ` Vladimir Davydov
  2018-04-02 10:27   ` [tarantool-patches] " v.shpilevoy
  2018-03-30 15:47 ` [PATCH 2/2] vinyl: zap upsert_format Vladimir Davydov
  1 sibling, 1 reply; 7+ messages in thread
From: Vladimir Davydov @ 2018-03-30 15:47 UTC (permalink / raw)
  To: kostja; +Cc: v.shpilevoy, tarantool-patches

Currently, we store upsert counter in tuple metadata (that's what
upsert_format is for), but since it's only relevant for tuples of
the memory level, we can store it on lsregion, right before tuple
data. Let's do it now so that we can get rid of upsert_format.
---
 src/box/vy_stmt.c | 24 ++++++++++++++++--------
 src/box/vy_stmt.h | 23 ++++++++++++++---------
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 84182e76..ac60c89e 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -134,13 +134,26 @@ struct tuple *
 vy_stmt_dup_lsregion(const struct tuple *stmt, struct lsregion *lsregion,
 		     int64_t alloc_id)
 {
+	enum iproto_type type = vy_stmt_type(stmt);
 	size_t size = tuple_size(stmt);
+	size_t alloc_size = size;
 	struct tuple *mem_stmt;
-	mem_stmt = lsregion_alloc(lsregion, size, alloc_id);
+
+	/* Reserve one byte for UPSERT counter. */
+	if (type == IPROTO_UPSERT)
+		alloc_size++;
+
+	mem_stmt = lsregion_alloc(lsregion, alloc_size, alloc_id);
 	if (mem_stmt == NULL) {
 		diag_set(OutOfMemory, size, "lsregion_alloc", "mem_stmt");
 		return NULL;
 	}
+
+	if (type == IPROTO_UPSERT) {
+		*(uint8_t *)mem_stmt = 0;
+		mem_stmt = (struct tuple *)((uint8_t *)mem_stmt + 1);
+	}
+
 	memcpy(mem_stmt, stmt, size);
 	/*
 	 * Region allocated statements can't be referenced or unreferenced
@@ -269,13 +282,8 @@ vy_stmt_new_upsert(struct tuple_format *format, const char *tuple_begin,
 	 * memory.
 	 */
 	assert(format->extra_size == sizeof(uint8_t));
-	struct tuple *upsert =
-		vy_stmt_new_with_ops(format, tuple_begin, tuple_end,
-				     operations, ops_cnt, IPROTO_UPSERT);
-	if (upsert == NULL)
-		return NULL;
-	vy_stmt_set_n_upserts(upsert, 0);
-	return upsert;
+	return vy_stmt_new_with_ops(format, tuple_begin, tuple_end,
+				    operations, ops_cnt, IPROTO_UPSERT);
 }
 
 struct tuple *
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index a33739d6..8958adb5 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -146,23 +146,28 @@ vy_stmt_set_type(struct tuple *stmt, enum iproto_type type)
 	((struct vy_stmt *) stmt)->type = type;
 }
 
-/** Get upserts count of the vinyl statement. */
+/**
+ * Get upserts count of the vinyl statement.
+ * Only for UPSERT statements allocated on lsregion.
+ */
 static inline uint8_t
 vy_stmt_n_upserts(const struct tuple *stmt)
 {
-	assert(tuple_format(stmt)->extra_size == sizeof(uint8_t));
-	return *((const uint8_t *) tuple_extra(stmt));
+	assert(stmt->refs == 0);
+	assert(vy_stmt_type(stmt) == IPROTO_UPSERT);
+	return *((uint8_t *)stmt - 1);
 }
 
-/** Set upserts count of the vinyl statement. */
+/**
+ * Set upserts count of the vinyl statement.
+ * Only for UPSERT statements allocated on lsregion.
+ */
 static inline void
 vy_stmt_set_n_upserts(struct tuple *stmt, uint8_t n)
 {
-	struct tuple_format *format = tuple_format(stmt);
-	assert(format->extra_size == sizeof(uint8_t));
-	char *extra = (char *) stmt + stmt->data_offset -
-		      tuple_format_meta_size(format);
-	*((uint8_t *) extra) = n;
+	assert(stmt->refs == 0);
+	assert(vy_stmt_type(stmt) == IPROTO_UPSERT);
+	*((uint8_t *)stmt - 1) = n;
 }
 
 /** Get the column mask of the specified tuple. */
-- 
2.11.0

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

* [PATCH 2/2] vinyl: zap upsert_format
  2018-03-30 15:47 [PATCH 0/2] vinyl: zap upsert_format Vladimir Davydov
  2018-03-30 15:47 ` [PATCH 1/2] vinyl: allocate upsert counter on lsregion Vladimir Davydov
@ 2018-03-30 15:47 ` Vladimir Davydov
  2018-04-02 10:58   ` v.shpilevoy
  1 sibling, 1 reply; 7+ messages in thread
From: Vladimir Davydov @ 2018-03-30 15:47 UTC (permalink / raw)
  To: kostja; +Cc: v.shpilevoy, tarantool-patches

The only difference between format of UPSERT statements and format of
other DML statements of the same index is that the former reserves one
byte for UPSERT counter, which is needed to schedule UPSERT squashing.
Since we store UPSERT counter on lsregion now, we don't need a special
format for UPSERTs anymore. Remove it.
---
 src/box/vinyl.c                 | 33 ++++------------------------
 src/box/vy_lsm.c                | 48 +++++++++++------------------------------
 src/box/vy_lsm.h                |  5 -----
 src/box/vy_mem.c                | 15 ++++---------
 src/box/vy_mem.h                |  6 +-----
 src/box/vy_point_lookup.c       |  6 ++----
 src/box/vy_read_iterator.c      |  5 ++---
 src/box/vy_run.c                | 33 +++++++++++-----------------
 src/box/vy_run.h                | 11 ++--------
 src/box/vy_scheduler.c          |  8 +++----
 src/box/vy_stmt.c               | 48 +++++------------------------------------
 src/box/vy_stmt.h               | 26 +---------------------
 src/box/vy_tx.c                 |  9 +++-----
 src/box/vy_upsert.c             | 11 +++++-----
 src/box/vy_upsert.h             |  3 +--
 src/box/vy_write_iterator.c     | 27 ++++++++---------------
 src/box/vy_write_iterator.h     |  8 +++----
 test/unit/vy_cache.c            | 11 +++++-----
 test/unit/vy_iterators_helper.c | 28 ++++++++----------------
 test/unit/vy_iterators_helper.h |  4 ----
 test/unit/vy_point_lookup.c     | 18 ++++++----------
 test/unit/vy_write_iterator.c   |  6 ++----
 22 files changed, 95 insertions(+), 274 deletions(-)

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 8abfaac9..ce51fdd0 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1023,14 +1023,6 @@ vinyl_space_commit_alter(struct space *old_space, struct space *new_space)
 	if (format == NULL)
 		goto fail;
 
-	/* Update the upsert format. */
-	struct tuple_format *upsert_format =
-		vy_tuple_format_new_upsert(new_format);
-	if (upsert_format == NULL) {
-		tuple_format_delete(format);
-		goto fail;
-	}
-
 	/* Set possibly changed opts. */
 	pk->opts = new_index_def->opts;
 	pk->check_is_unique = true;
@@ -1038,12 +1030,9 @@ vinyl_space_commit_alter(struct space *old_space, struct space *new_space)
 	/* Set new formats. */
 	tuple_format_unref(pk->disk_format);
 	tuple_format_unref(pk->mem_format);
-	tuple_format_unref(pk->upsert_format);
 	tuple_format_unref(pk->mem_format_with_colmask);
 	pk->disk_format = new_format;
 	tuple_format_ref(new_format);
-	pk->upsert_format = upsert_format;
-	tuple_format_ref(upsert_format);
 	pk->mem_format_with_colmask = format;
 	tuple_format_ref(format);
 	pk->mem_format = new_format;
@@ -1062,13 +1051,10 @@ vinyl_space_commit_alter(struct space *old_space, struct space *new_space)
 		lsm->check_is_unique = lsm->opts.is_unique;
 		tuple_format_unref(lsm->mem_format_with_colmask);
 		tuple_format_unref(lsm->mem_format);
-		tuple_format_unref(lsm->upsert_format);
 		lsm->mem_format_with_colmask = pk->mem_format_with_colmask;
 		lsm->mem_format = pk->mem_format;
-		lsm->upsert_format = pk->upsert_format;
 		tuple_format_ref(lsm->mem_format_with_colmask);
 		tuple_format_ref(lsm->mem_format);
-		tuple_format_ref(lsm->upsert_format);
 		key_def_update_optionality(lsm->key_def,
 					   new_format->min_field_count);
 		key_def_update_optionality(lsm->cmp_def,
@@ -1919,7 +1905,7 @@ vy_lsm_upsert(struct vy_tx *tx, struct vy_lsm *lsm,
 	struct iovec operations[1];
 	operations[0].iov_base = (void *)expr;
 	operations[0].iov_len = expr_end - expr;
-	vystmt = vy_stmt_new_upsert(lsm->upsert_format, tuple, tuple_end,
+	vystmt = vy_stmt_new_upsert(lsm->mem_format, tuple, tuple_end,
 				    operations, 1);
 	if (vystmt == NULL)
 		return -1;
@@ -2896,8 +2882,6 @@ struct vy_join_ctx {
 	struct key_def *key_def;
 	/** LSM tree format used for REPLACE and DELETE statements. */
 	struct tuple_format *format;
-	/** LSM tree format used for UPSERT statements. */
-	struct tuple_format *upsert_format;
 	/**
 	 * Write iterator for merging runs before sending
 	 * them to the replica.
@@ -3014,8 +2998,7 @@ vy_send_range(struct vy_join_ctx *ctx,
 	/* Create a write iterator. */
 	struct rlist fake_read_views;
 	rlist_create(&fake_read_views);
-	ctx->wi = vy_write_iterator_new(ctx->key_def,
-					ctx->format, ctx->upsert_format,
+	ctx->wi = vy_write_iterator_new(ctx->key_def, ctx->format,
 					true, true, &fake_read_views);
 	if (ctx->wi == NULL) {
 		rc = -1;
@@ -3072,10 +3055,6 @@ vy_send_lsm(struct vy_join_ctx *ctx, struct vy_lsm_recovery_info *lsm_info)
 	if (ctx->format == NULL)
 		goto out_free_key_def;
 	tuple_format_ref(ctx->format);
-	ctx->upsert_format = vy_tuple_format_new_upsert(ctx->format);
-	if (ctx->upsert_format == NULL)
-		goto out_free_format;
-	tuple_format_ref(ctx->upsert_format);
 
 	/* Send ranges. */
 	struct vy_range_recovery_info *range_info;
@@ -3086,9 +3065,6 @@ vy_send_lsm(struct vy_join_ctx *ctx, struct vy_lsm_recovery_info *lsm_info)
 			break;
 	}
 
-	tuple_format_unref(ctx->upsert_format);
-	ctx->upsert_format = NULL;
-out_free_format:
 	tuple_format_unref(ctx->format);
 	ctx->format = NULL;
 out_free_key_def:
@@ -3543,9 +3519,8 @@ vy_squash_process(struct vy_squash *squash)
 			return 0;
 		}
 		assert(lsm->index_id == 0);
-		struct tuple *applied =
-			vy_apply_upsert(mem_stmt, result, def, mem->format,
-					mem->upsert_format, true);
+		struct tuple *applied = vy_apply_upsert(mem_stmt, result, def,
+							mem->format, true);
 		lsm->stat.upsert.applied++;
 		tuple_unref(result);
 		if (applied == NULL)
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 88a75e32..4b2c3ef3 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -59,7 +59,6 @@ vy_lsm_validate_formats(const struct vy_lsm *lsm)
 	assert(lsm->disk_format != NULL);
 	assert(lsm->mem_format != NULL);
 	assert(lsm->mem_format_with_colmask != NULL);
-	assert(lsm->upsert_format != NULL);
 	uint32_t index_field_count = lsm->mem_format->index_field_count;
 	(void) index_field_count;
 	if (lsm->index_id == 0) {
@@ -73,7 +72,6 @@ vy_lsm_validate_formats(const struct vy_lsm *lsm)
 		assert(lsm->disk_format->index_field_count <=
 		       index_field_count);
 	}
-	assert(lsm->upsert_format->index_field_count == index_field_count);
 	assert(lsm->mem_format_with_colmask->index_field_count ==
 	       index_field_count);
 }
@@ -189,12 +187,6 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 	tuple_format_ref(lsm->disk_format);
 
 	if (index_def->iid == 0) {
-		lsm->upsert_format =
-			vy_tuple_format_new_upsert(format);
-		if (lsm->upsert_format == NULL)
-			goto fail_upsert_format;
-		tuple_format_ref(lsm->upsert_format);
-
 		lsm->mem_format_with_colmask =
 			vy_tuple_format_new_with_colmask(format);
 		if (lsm->mem_format_with_colmask == NULL)
@@ -202,9 +194,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 		tuple_format_ref(lsm->mem_format_with_colmask);
 	} else {
 		lsm->mem_format_with_colmask = pk->mem_format_with_colmask;
-		lsm->upsert_format = pk->upsert_format;
 		tuple_format_ref(lsm->mem_format_with_colmask);
-		tuple_format_ref(lsm->upsert_format);
 	}
 
 	if (vy_lsm_stat_create(&lsm->stat) != 0)
@@ -216,7 +206,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
 
 	lsm->mem = vy_mem_new(mem_env, *lsm->env->p_generation,
 			      cmp_def, format, lsm->mem_format_with_colmask,
-			      lsm->upsert_format, schema_version);
+			      schema_version);
 	if (lsm->mem == NULL)
 		goto fail_mem;
 
@@ -252,8 +242,6 @@ fail_run_hist:
 fail_stat:
 	tuple_format_unref(lsm->mem_format_with_colmask);
 fail_mem_format_with_colmask:
-	tuple_format_unref(lsm->upsert_format);
-fail_upsert_format:
 	tuple_format_unref(lsm->disk_format);
 fail_format:
 	key_def_delete(cmp_def);
@@ -306,7 +294,6 @@ vy_lsm_delete(struct vy_lsm *lsm)
 	vy_range_heap_destroy(&lsm->range_heap);
 	tuple_format_unref(lsm->disk_format);
 	tuple_format_unref(lsm->mem_format_with_colmask);
-	tuple_format_unref(lsm->upsert_format);
 	key_def_delete(lsm->cmp_def);
 	key_def_delete(lsm->key_def);
 	histogram_delete(lsm->run_hist);
@@ -397,8 +384,7 @@ vy_lsm_recover_run(struct vy_lsm *lsm, struct vy_run_recovery_info *run_info,
 	     vy_run_rebuild_index(run, lsm->env->path,
 				  lsm->space_id, lsm->index_id,
 				  lsm->cmp_def, lsm->key_def,
-				  lsm->mem_format, lsm->upsert_format,
-				  &lsm->opts) != 0)) {
+				  lsm->mem_format, &lsm->opts) != 0)) {
 		vy_run_unref(run);
 		return NULL;
 	}
@@ -759,8 +745,7 @@ vy_lsm_rotate_mem(struct vy_lsm *lsm)
 	assert(lsm->mem != NULL);
 	mem = vy_mem_new(lsm->mem->env, *lsm->env->p_generation,
 			 lsm->cmp_def, lsm->mem_format,
-			 lsm->mem_format_with_colmask,
-			 lsm->upsert_format, schema_version);
+			 lsm->mem_format_with_colmask, schema_version);
 	if (mem == NULL)
 		return -1;
 
@@ -798,23 +783,17 @@ vy_lsm_set(struct vy_lsm *lsm, struct vy_mem *mem,
 	/* We can't free region_stmt below, so let's add it to the stats */
 	lsm->stat.memory.count.bytes += tuple_size(stmt);
 
+	/* Abort transaction if format was changed by DDL */
 	uint32_t format_id = stmt->format_id;
-	if (vy_stmt_type(*region_stmt) != IPROTO_UPSERT) {
-		/* Abort transaction if format was changed by DDL */
-		if (format_id != tuple_format_id(mem->format_with_colmask) &&
-		    format_id != tuple_format_id(mem->format)) {
-			diag_set(ClientError, ER_TRANSACTION_CONFLICT);
-			return -1;
-		}
+	if (format_id != tuple_format_id(mem->format_with_colmask) &&
+	    format_id != tuple_format_id(mem->format)) {
+		diag_set(ClientError, ER_TRANSACTION_CONFLICT);
+		return -1;
+	}
+	if (vy_stmt_type(*region_stmt) != IPROTO_UPSERT)
 		return vy_mem_insert(mem, *region_stmt);
-	} else {
-		/* Abort transaction if format was changed by DDL */
-		if (format_id != tuple_format_id(mem->upsert_format)) {
-			diag_set(ClientError, ER_TRANSACTION_CONFLICT);
-			return -1;
-		}
+	else
 		return vy_mem_insert_upsert(mem, *region_stmt);
-	}
 }
 
 /**
@@ -875,7 +854,7 @@ vy_lsm_commit_upsert(struct vy_lsm *lsm, struct vy_mem *mem,
 			return;
 		}
 
-		struct tuple *dup = vy_stmt_dup(stmt, lsm->upsert_format);
+		struct tuple *dup = vy_stmt_dup(stmt, lsm->mem_format);
 		if (dup != NULL) {
 			lsm->env->upsert_thresh_cb(lsm, dup,
 					lsm->env->upsert_thresh_arg);
@@ -899,8 +878,7 @@ vy_lsm_commit_upsert(struct vy_lsm *lsm, struct vy_mem *mem,
 		assert(older == NULL || vy_stmt_type(older) != IPROTO_UPSERT);
 		struct tuple *upserted =
 			vy_apply_upsert(stmt, older, lsm->cmp_def,
-					lsm->mem_format,
-					lsm->upsert_format, false);
+					lsm->mem_format, false);
 		lsm->stat.upsert.applied++;
 
 		if (upserted == NULL) {
diff --git a/src/box/vy_lsm.h b/src/box/vy_lsm.h
index dcfbcb7b..ab54da75 100644
--- a/src/box/vy_lsm.h
+++ b/src/box/vy_lsm.h
@@ -193,11 +193,6 @@ struct vy_lsm {
 	 * tuples for vy_mem, and used only by primary key.
 	 */
 	struct tuple_format *mem_format_with_colmask;
-	/*
-	 * Format for UPSERT statements. Note, UPSERTs can only
-	 * appear in spaces with a single index.
-	 */
-	struct tuple_format *upsert_format;
 	/**
 	 * If this LSM tree is for a secondary index, the following
 	 * variable points to the LSM tree of the primary index of
diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
index 1105340c..68abf5bc 100644
--- a/src/box/vy_mem.c
+++ b/src/box/vy_mem.c
@@ -97,8 +97,7 @@ vy_mem_tree_extent_free(void *ctx, void *p)
 struct vy_mem *
 vy_mem_new(struct vy_mem_env *env, int64_t generation,
 	   const struct key_def *cmp_def, struct tuple_format *format,
-	   struct tuple_format *format_with_colmask,
-	   struct tuple_format *upsert_format, uint32_t schema_version)
+	   struct tuple_format *format_with_colmask, uint32_t schema_version)
 {
 	struct vy_mem *index = calloc(1, sizeof(*index));
 	if (!index) {
@@ -116,8 +115,6 @@ vy_mem_new(struct vy_mem_env *env, int64_t generation,
 	tuple_format_ref(format);
 	index->format_with_colmask = format_with_colmask;
 	tuple_format_ref(format_with_colmask);
-	index->upsert_format = upsert_format;
-	tuple_format_ref(upsert_format);
 	vy_mem_tree_create(&index->tree, cmp_def,
 			   vy_mem_tree_extent_alloc,
 			   vy_mem_tree_extent_free, index);
@@ -128,19 +125,15 @@ vy_mem_new(struct vy_mem_env *env, int64_t generation,
 
 void
 vy_mem_update_formats(struct vy_mem *mem, struct tuple_format *new_format,
-		      struct tuple_format *new_format_with_colmask,
-		      struct tuple_format *new_upsert_format)
+		      struct tuple_format *new_format_with_colmask)
 {
 	assert(mem->count.rows == 0);
 	tuple_format_unref(mem->format);
 	tuple_format_unref(mem->format_with_colmask);
-	tuple_format_unref(mem->upsert_format);
 	mem->format = new_format;
 	mem->format_with_colmask = new_format_with_colmask;
-	mem->upsert_format = new_upsert_format;
 	tuple_format_ref(mem->format);
 	tuple_format_ref(mem->format_with_colmask);
-	tuple_format_ref(mem->upsert_format);
 }
 
 void
@@ -149,7 +142,6 @@ vy_mem_delete(struct vy_mem *index)
 	index->env->tree_extent_size -= index->tree_extent_size;
 	tuple_format_unref(index->format);
 	tuple_format_unref(index->format_with_colmask);
-	tuple_format_unref(index->upsert_format);
 	fiber_cond_destroy(&index->pin_cond);
 	TRASH(index);
 	free(index);
@@ -180,7 +172,8 @@ vy_mem_insert_upsert(struct vy_mem *mem, const struct tuple *stmt)
 {
 	assert(vy_stmt_type(stmt) == IPROTO_UPSERT);
 	/* Check if the statement can be inserted in the vy_mem. */
-	assert(stmt->format_id == tuple_format_id(mem->upsert_format));
+	assert(stmt->format_id == tuple_format_id(mem->format_with_colmask) ||
+	       stmt->format_id == tuple_format_id(mem->format));
 	/* The statement must be from a lsregion. */
 	assert(!vy_stmt_is_refable(stmt));
 	size_t size = tuple_size(stmt);
diff --git a/src/box/vy_mem.h b/src/box/vy_mem.h
index d6003221..c2917efb 100644
--- a/src/box/vy_mem.h
+++ b/src/box/vy_mem.h
@@ -188,8 +188,6 @@ struct vy_mem {
 	struct tuple_format *format;
 	/** Format of vy_mem tuples with column mask. */
 	struct tuple_format *format_with_colmask;
-	/** Same as format, but for UPSERT tuples. */
-	struct tuple_format *upsert_format;
 	/**
 	 * Number of active writers to this index.
 	 *
@@ -249,7 +247,6 @@ vy_mem_wait_pinned(struct vy_mem *mem)
  * @param format Format for REPLACE and DELETE tuples.
  * @param format_with_colmask Format for tuples, which have
  *        column mask.
- * @param upsert_format Format for UPSERT tuples.
  * @param schema_version Schema version.
  * @retval new vy_mem instance on success.
  * @retval NULL on error, check diag.
@@ -257,8 +254,7 @@ vy_mem_wait_pinned(struct vy_mem *mem)
 struct vy_mem *
 vy_mem_new(struct vy_mem_env *env, int64_t generation,
 	   const struct key_def *cmp_def, struct tuple_format *format,
-	   struct tuple_format *format_with_colmask,
-	   struct tuple_format *upsert_format, uint32_t schema_version);
+	   struct tuple_format *format_with_colmask, uint32_t schema_version);
 
 /**
  * Delete in-memory level.
diff --git a/src/box/vy_point_lookup.c b/src/box/vy_point_lookup.c
index c5aa9719..0f4d75f7 100644
--- a/src/box/vy_point_lookup.c
+++ b/src/box/vy_point_lookup.c
@@ -265,8 +265,7 @@ vy_point_lookup_scan_slice(struct vy_lsm *lsm, struct vy_slice *slice,
 	struct vy_run_iterator run_itr;
 	vy_run_iterator_open(&run_itr, &lsm->stat.disk.iterator, slice,
 			     ITER_EQ, key, rv, lsm->cmp_def, lsm->key_def,
-			     lsm->disk_format, lsm->upsert_format,
-			     lsm->index_id == 0);
+			     lsm->disk_format, lsm->index_id == 0);
 	struct tuple *stmt;
 	rc = vy_run_iterator_next_key(&run_itr, &stmt);
 	while (rc == 0 && stmt != NULL) {
@@ -363,8 +362,7 @@ vy_point_lookup_apply_history(struct vy_lsm *lsm,
 		       vy_stmt_lsn(node->stmt) <= (*rv)->vlsn);
 
 		struct tuple *stmt = vy_apply_upsert(node->stmt, curr_stmt,
-					lsm->cmp_def, lsm->mem_format,
-					lsm->upsert_format, true);
+					lsm->cmp_def, lsm->mem_format, true);
 		lsm->stat.upsert.applied++;
 		if (stmt == NULL)
 			return -1;
diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
index 37493e19..2cad2337 100644
--- a/src/box/vy_read_iterator.c
+++ b/src/box/vy_read_iterator.c
@@ -659,8 +659,7 @@ vy_read_iterator_squash_upsert(struct vy_read_iterator *itr,
 			return rc;
 		}
 		struct tuple *applied = vy_apply_upsert(t, next,
-				lsm->cmp_def, lsm->mem_format,
-				lsm->upsert_format, true);
+				lsm->cmp_def, lsm->mem_format, true);
 		lsm->stat.upsert.applied++;
 		tuple_unref(t);
 		if (applied == NULL)
@@ -753,7 +752,7 @@ vy_read_iterator_add_disk(struct vy_read_iterator *itr)
 				     iterator_type, itr->key,
 				     itr->read_view, lsm->cmp_def,
 				     lsm->key_def, lsm->disk_format,
-				     lsm->upsert_format, lsm->index_id == 0);
+				     lsm->index_id == 0);
 	}
 }
 
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index da6d5872..637f63fa 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -682,7 +682,6 @@ vy_page_xrow(struct vy_page *page, uint32_t stmt_no,
  * @param stmt_no       Statement position in the page.
  * @param cmp_def       Key definition, including primary key parts.
  * @param format        Format for REPLACE/DELETE tuples.
- * @param upsert_format Format for UPSERT tuples.
  * @param is_primary    True if the index is primary.
  *
  * @retval not NULL Statement read from page.
@@ -691,12 +690,12 @@ vy_page_xrow(struct vy_page *page, uint32_t stmt_no,
 static struct tuple *
 vy_page_stmt(struct vy_page *page, uint32_t stmt_no,
 	     const struct key_def *cmp_def, struct tuple_format *format,
-	     struct tuple_format *upsert_format, bool is_primary)
+	     bool is_primary)
 {
 	struct xrow_header xrow;
 	if (vy_page_xrow(page, stmt_no, &xrow) != 0)
 		return NULL;
-	return vy_stmt_decode(&xrow, cmp_def, format, upsert_format, is_primary);
+	return vy_stmt_decode(&xrow, cmp_def, format, is_primary);
 }
 
 /**
@@ -993,7 +992,7 @@ vy_run_iterator_read(struct vy_run_iterator *itr,
 	if (rc != 0)
 		return rc;
 	*stmt = vy_page_stmt(page, pos.pos_in_page, itr->cmp_def,
-			     itr->format, itr->upsert_format, itr->is_primary);
+			     itr->format, itr->is_primary);
 	if (*stmt == NULL)
 		return -1;
 	return 0;
@@ -1020,7 +1019,7 @@ vy_run_iterator_search_in_page(struct vy_run_iterator *itr,
 	while (beg != end) {
 		uint32_t mid = beg + (end - beg) / 2;
 		struct tuple *fnd_key = vy_page_stmt(page, mid, itr->cmp_def,
-						     itr->format, itr->upsert_format,
+						     itr->format,
 						     itr->is_primary);
 		if (fnd_key == NULL)
 			return end;
@@ -1370,14 +1369,12 @@ vy_run_iterator_open(struct vy_run_iterator *itr,
 		     const struct key_def *cmp_def,
 		     const struct key_def *key_def,
 		     struct tuple_format *format,
-		     struct tuple_format *upsert_format,
 		     bool is_primary)
 {
 	itr->stat = stat;
 	itr->cmp_def = cmp_def;
 	itr->key_def = key_def;
 	itr->format = format;
-	itr->upsert_format = upsert_format;
 	itr->is_primary = is_primary;
 	itr->slice = slice;
 
@@ -2205,7 +2202,6 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
 		     const struct key_def *cmp_def,
 		     const struct key_def *key_def,
 		     struct tuple_format *mem_format,
-		     struct tuple_format *upsert_format,
 		     const struct index_opts *opts)
 {
 	assert(run->info.bloom == NULL);
@@ -2259,7 +2255,7 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
 			}
 			++page_row_count;
 			struct tuple *tuple = vy_stmt_decode(&xrow, cmp_def,
-					mem_format, upsert_format, iid == 0);
+							mem_format, iid == 0);
 			if (tuple == NULL)
 				goto close_err;
 			if (bloom_builder != NULL) {
@@ -2427,10 +2423,9 @@ vy_slice_stream_search(struct vy_stmt_stream *virt_stream)
 	uint32_t end = stream->page->row_count;
 	while (beg != end) {
 		uint32_t mid = beg + (end - beg) / 2;
-		struct tuple *fnd_key =
-			vy_page_stmt(stream->page, mid, stream->cmp_def,
-				     stream->format, stream->upsert_format,
-				     stream->is_primary);
+		struct tuple *fnd_key = vy_page_stmt(stream->page, mid,
+					stream->cmp_def, stream->format,
+					stream->is_primary);
 		if (fnd_key == NULL)
 			return -1;
 		int cmp = vy_tuple_compare_with_key(fnd_key,
@@ -2476,10 +2471,9 @@ vy_slice_stream_next(struct vy_stmt_stream *virt_stream, struct tuple **ret)
 		return -1;
 
 	/* Read current tuple from the page */
-	struct tuple *tuple =
-		vy_page_stmt(stream->page, stream->pos_in_page,
-			     stream->cmp_def, stream->format,
-			     stream->upsert_format, stream->is_primary);
+	struct tuple *tuple = vy_page_stmt(stream->page, stream->pos_in_page,
+					   stream->cmp_def, stream->format,
+					   stream->is_primary);
 	if (tuple == NULL) /* Read or memory error */
 		return -1;
 
@@ -2543,8 +2537,8 @@ static const struct vy_stmt_stream_iface vy_slice_stream_iface = {
 
 void
 vy_slice_stream_open(struct vy_slice_stream *stream, struct vy_slice *slice,
-		   const struct key_def *cmp_def, struct tuple_format *format,
-		   struct tuple_format *upsert_format, bool is_primary)
+		     const struct key_def *cmp_def, struct tuple_format *format,
+		     bool is_primary)
 {
 	stream->base.iface = &vy_slice_stream_iface;
 
@@ -2556,6 +2550,5 @@ vy_slice_stream_open(struct vy_slice_stream *stream, struct vy_slice *slice,
 	stream->slice = slice;
 	stream->cmp_def = cmp_def;
 	stream->format = format;
-	stream->upsert_format = upsert_format;
 	stream->is_primary = is_primary;
 }
diff --git a/src/box/vy_run.h b/src/box/vy_run.h
index e1d7101b..1e8bf740 100644
--- a/src/box/vy_run.h
+++ b/src/box/vy_run.h
@@ -220,8 +220,6 @@ struct vy_run_iterator {
 	 * pages.
 	 */
 	struct tuple_format *format;
-	/** Same as format, but for UPSERT tuples. */
-	struct tuple_format *upsert_format;
 	/** Set if this iterator is for a primary index. */
 	bool is_primary;
 	/** The run slice to iterate. */
@@ -367,7 +365,6 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
 		     const struct key_def *key_def,
 		     const struct key_def *user_key_def,
 		     struct tuple_format *mem_format,
-		     struct tuple_format *upsert_format,
 		     const struct index_opts *opts);
 
 enum vy_file_type {
@@ -491,9 +488,7 @@ vy_run_iterator_open(struct vy_run_iterator *itr,
 		     const struct tuple *key, const struct vy_read_view **rv,
 		     const struct key_def *cmp_def,
 		     const struct key_def *key_def,
-		     struct tuple_format *format,
-		     struct tuple_format *upsert_format,
-		     bool is_primary);
+		     struct tuple_format *format, bool is_primary);
 
 /**
  * Advance a run iterator to the newest statement for the next key.
@@ -551,8 +546,6 @@ struct vy_slice_stream {
 	const struct key_def *cmp_def;
 	/** Format for allocating REPLACE and DELETE tuples read from pages. */
 	struct tuple_format *format;
-	/** Same as format, but for UPSERT tuples. */
-	struct tuple_format *upsert_format;
 	/** Set if this iterator is for a primary index. */
 	bool is_primary;
 };
@@ -563,7 +556,7 @@ struct vy_slice_stream {
 void
 vy_slice_stream_open(struct vy_slice_stream *stream, struct vy_slice *slice,
 		     const struct key_def *cmp_def, struct tuple_format *format,
-		     struct tuple_format *upsert_format, bool is_primary);
+		     bool is_primary);
 
 /**
  * Run_writer fills a created run with statements one by one,
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 0bf9ce77..369b963f 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -996,8 +996,8 @@ vy_task_dump_new(struct vy_scheduler *scheduler, struct vy_lsm *lsm,
 	struct vy_stmt_stream *wi;
 	bool is_last_level = (lsm->run_count == 0);
 	wi = vy_write_iterator_new(task->cmp_def, lsm->disk_format,
-				   lsm->upsert_format, lsm->index_id == 0,
-				   is_last_level, scheduler->read_views);
+				   lsm->index_id == 0, is_last_level,
+				   scheduler->read_views);
 	if (wi == NULL)
 		goto err_wi;
 	rlist_foreach_entry(mem, &lsm->sealed, in_sealed) {
@@ -1268,8 +1268,8 @@ vy_task_compact_new(struct vy_scheduler *scheduler, struct vy_lsm *lsm,
 	struct vy_stmt_stream *wi;
 	bool is_last_level = (range->compact_priority == range->slice_count);
 	wi = vy_write_iterator_new(task->cmp_def, lsm->disk_format,
-				   lsm->upsert_format, lsm->index_id == 0,
-				   is_last_level, scheduler->read_views);
+				   lsm->index_id == 0, is_last_level,
+				   scheduler->read_views);
 	if (wi == NULL)
 		goto err_wi;
 
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index ac60c89e..f65b63c5 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -116,8 +116,6 @@ vy_stmt_dup(const struct tuple *stmt, struct tuple_format *format)
 	 * tuple field map. This map can be simple memcopied from
 	 * the original tuple.
 	 */
-	assert((vy_stmt_type(stmt) == IPROTO_UPSERT) ==
-	       (format->extra_size == sizeof(uint8_t)));
 	struct tuple *res = vy_stmt_alloc(format, stmt->bsize);
 	if (res == NULL)
 		return NULL;
@@ -184,8 +182,6 @@ vy_stmt_new_select(struct tuple_format *format, const char *key,
 	assert(part_count == 0 || key != NULL);
 	/* Key don't have field map */
 	assert(format->field_map_size == 0);
-	/* Key doesn't have n_upserts field. */
-	assert(format->extra_size != sizeof(uint8_t));
 
 	/* Calculate key length */
 	const char *key_end = key;
@@ -277,11 +273,6 @@ vy_stmt_new_upsert(struct tuple_format *format, const char *tuple_begin,
 		   const char *tuple_end, struct iovec *operations,
 		   uint32_t ops_cnt)
 {
-	/*
-	 * UPSERT must have the n_upserts field in the extra
-	 * memory.
-	 */
-	assert(format->extra_size == sizeof(uint8_t));
 	return vy_stmt_new_with_ops(format, tuple_begin, tuple_end,
 				    operations, ops_cnt, IPROTO_UPSERT);
 }
@@ -290,8 +281,6 @@ struct tuple *
 vy_stmt_new_replace(struct tuple_format *format, const char *tuple_begin,
 		    const char *tuple_end)
 {
-	/* REPLACE mustn't have n_upserts field. */
-	assert(format->extra_size != sizeof(uint8_t));
 	return vy_stmt_new_with_ops(format, tuple_begin, tuple_end,
 				    NULL, 0, IPROTO_REPLACE);
 }
@@ -300,8 +289,6 @@ struct tuple *
 vy_stmt_new_insert(struct tuple_format *format, const char *tuple_begin,
 		   const char *tuple_end)
 {
-	/* INSERT mustn't have n_upserts field. */
-	assert(format->extra_size != sizeof(uint8_t));
 	return vy_stmt_new_with_ops(format, tuple_begin, tuple_end,
 				    NULL, 0, IPROTO_INSERT);
 }
@@ -310,8 +297,6 @@ struct tuple *
 vy_stmt_replace_from_upsert(struct tuple_format *replace_format,
 			    const struct tuple *upsert)
 {
-	/* REPLACE mustn't have n_upserts field. */
-	assert(replace_format->extra_size == 0);
 	assert(vy_stmt_type(upsert) == IPROTO_UPSERT);
 	/* Get statement size without UPSERT operations */
 	uint32_t bsize;
@@ -321,11 +306,6 @@ vy_stmt_replace_from_upsert(struct tuple_format *replace_format,
 	/* Copy statement data excluding UPSERT operations */
 	struct tuple_format *format = tuple_format_by_id(upsert->format_id);
 	/*
-	 * UPSERT must have the n_upserts field in the extra
-	 * memory.
-	 */
-	assert(format->extra_size == sizeof(uint8_t));
-	/*
 	 * In other fields the REPLACE tuple format must equal to
 	 * the UPSERT tuple format.
 	 */
@@ -335,8 +315,7 @@ vy_stmt_replace_from_upsert(struct tuple_format *replace_format,
 		return NULL;
 	/* Copy both data and field_map. */
 	char *dst = (char *)replace + sizeof(struct vy_stmt);
-	char *src = (char *)upsert + sizeof(struct vy_stmt) +
-		    format->extra_size;
+	char *src = (char *)upsert + sizeof(struct vy_stmt);
 	memcpy(dst, src, format->field_map_size + bsize);
 	vy_stmt_set_type(replace, IPROTO_REPLACE);
 	vy_stmt_set_lsn(replace, vy_stmt_lsn(upsert));
@@ -348,11 +327,8 @@ vy_stmt_new_surrogate_from_key(const char *key, enum iproto_type type,
 			       const struct key_def *cmp_def,
 			       struct tuple_format *format)
 {
-	/**
-	 * UPSERT can't be surrogate. Also any not UPSERT tuple
-	 * mustn't have the n_upserts field.
-	 */
-	assert(type != IPROTO_UPSERT && format->extra_size != sizeof(uint8_t));
+	/* UPSERT can't be surrogate. */
+	assert(type != IPROTO_UPSERT);
 	struct region *region = &fiber()->gc;
 
 	uint32_t field_count = format->index_field_count;
@@ -590,9 +566,7 @@ vy_stmt_encode_secondary(const struct tuple *value,
 
 struct tuple *
 vy_stmt_decode(struct xrow_header *xrow, const struct key_def *key_def,
-	       struct tuple_format *format,
-	       struct tuple_format *upsert_format,
-	       bool is_primary)
+	       struct tuple_format *format, bool is_primary)
 {
 	struct request request;
 	uint64_t key_map = dml_request_key_map(xrow->type);
@@ -625,7 +599,7 @@ vy_stmt_decode(struct xrow_header *xrow, const struct key_def *key_def,
 	case IPROTO_UPSERT:
 		ops.iov_base = (char *)request.ops;
 		ops.iov_len = request.ops_end - request.ops;
-		stmt = vy_stmt_new_upsert(upsert_format, request.tuple,
+		stmt = vy_stmt_new_upsert(format, request.tuple,
 					  request.tuple_end, &ops, 1);
 		break;
 	default:
@@ -686,15 +660,3 @@ vy_tuple_format_new_with_colmask(struct tuple_format *mem_format)
 	format->extra_size = sizeof(uint64_t);
 	return format;
 }
-
-struct tuple_format *
-vy_tuple_format_new_upsert(struct tuple_format *mem_format)
-{
-	struct tuple_format *format = tuple_format_dup(mem_format);
-	if (format == NULL)
-		return NULL;
-	/* + size of n_upserts. */
-	assert(format->extra_size == 0);
-	format->extra_size = sizeof(uint8_t);
-	return format;
-}
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 8958adb5..9e73b2ca 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -104,14 +104,6 @@ struct vy_stmt {
 	int64_t lsn;
 	uint8_t  type; /* IPROTO_SELECT/REPLACE/UPSERT/DELETE */
 	/**
-	 * Number of UPSERT statements for the same key preceding
-	 * this statement. Used to trigger upsert squashing in the
-	 * background (see vy_range_set_upsert()). This member is
-	 * stored only for UPSERT statements in the extra memory
-	 * space before offsets table.
-	 *
-	 *     uint8_t n_upserts;
-	 *
 	 * Offsets array concatenated with MessagePack fields
 	 * array.
 	 * char raw[0];
@@ -532,8 +524,6 @@ static inline const char *
 vy_upsert_data_range(const struct tuple *tuple, uint32_t *p_size)
 {
 	assert(vy_stmt_type(tuple) == IPROTO_UPSERT);
-	/* UPSERT must have the n_upserts field. */
-	assert(tuple_format(tuple)->extra_size == sizeof(uint8_t));
 	const char *mp = tuple_data(tuple);
 	assert(mp_typeof(*mp) == MP_ARRAY);
 	const char *mp_end = mp;
@@ -639,9 +629,7 @@ vy_stmt_encode_secondary(const struct tuple *value,
  */
 struct tuple *
 vy_stmt_decode(struct xrow_header *xrow, const struct key_def *key_def,
-	       struct tuple_format *format,
-	       struct tuple_format *upsert_format,
-	       bool is_primary);
+	       struct tuple_format *format, bool is_primary);
 
 /**
  * Format a statement into string.
@@ -670,18 +658,6 @@ struct tuple_format *
 vy_tuple_format_new_with_colmask(struct tuple_format *mem_format);
 
 /**
- * Create a tuple format for UPSERT tuples. UPSERTs has an additional
- * extra byte before an offsets table, that stores the count
- * of squashed upserts @sa vy_squash.
- * @param mem_format A base tuple format.
- *
- * @retval not NULL Success.
- * @retval     NULL Memory or format register error.
- */
-struct tuple_format *
-vy_tuple_format_new_upsert(struct tuple_format *mem_format);
-
-/**
  * Check if a key of @a tuple contains NULL.
  * @param tuple Tuple to check.
  * @param def Key def to check by.
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index ed423ba3..285af8a6 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -460,10 +460,8 @@ vy_tx_write(struct vy_lsm *lsm, struct vy_mem *mem,
 		/* Invalidate cache element. */
 		vy_cache_on_write(&lsm->cache, stmt, &deleted);
 		if (deleted != NULL) {
-			struct tuple *applied =
-				vy_apply_upsert(stmt, deleted, mem->cmp_def,
-						mem->format, mem->upsert_format,
-						false);
+			struct tuple *applied = vy_apply_upsert(stmt, deleted,
+					mem->cmp_def, mem->format, false);
 			tuple_unref(deleted);
 			if (applied != NULL) {
 				assert(vy_stmt_type(applied) == IPROTO_REPLACE);
@@ -823,8 +821,7 @@ vy_tx_set(struct vy_tx *tx, struct vy_lsm *lsm, struct tuple *stmt)
 		(void) old_type;
 
 		applied = vy_apply_upsert(stmt, old->stmt, lsm->cmp_def,
-					  lsm->mem_format,
-					  lsm->upsert_format, true);
+					  lsm->mem_format, true);
 		lsm->stat.upsert.applied++;
 		if (applied == NULL)
 			return -1;
diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c
index d86972b6..2b38139a 100644
--- a/src/box/vy_upsert.c
+++ b/src/box/vy_upsert.c
@@ -125,7 +125,7 @@ vy_upsert_try_to_squash(struct tuple_format *format, struct region *region,
 struct tuple *
 vy_apply_upsert(const struct tuple *new_stmt, const struct tuple *old_stmt,
 		const struct key_def *cmp_def, struct tuple_format *format,
-		struct tuple_format *upsert_format, bool suppress_error)
+		bool suppress_error)
 {
 	/*
 	 * old_stmt - previous (old) version of stmt
@@ -195,7 +195,7 @@ vy_apply_upsert(const struct tuple *new_stmt, const struct tuple *old_stmt,
 	 * UPSERT + UPSERT case: combine operations
 	 */
 	assert(old_ops_end - old_ops > 0);
-	if (vy_upsert_try_to_squash(upsert_format, region,
+	if (vy_upsert_try_to_squash(format, region,
 				    result_mp, result_mp_end,
 				    old_ops, old_ops_end,
 				    new_ops, new_ops_end,
@@ -226,8 +226,8 @@ vy_apply_upsert(const struct tuple *new_stmt, const struct tuple *old_stmt,
 	operations[0].iov_base = (void *)ops_buf;
 	operations[0].iov_len = header - ops_buf;
 
-	result_stmt = vy_stmt_new_upsert(upsert_format, result_mp,
-					 result_mp_end, operations, 3);
+	result_stmt = vy_stmt_new_upsert(format, result_mp, result_mp_end,
+					 operations, 3);
 	region_truncate(region, region_svp);
 	if (result_stmt == NULL)
 		return NULL;
@@ -244,8 +244,7 @@ check_key:
 		 * @retval the old stmt.
 		 */
 		tuple_unref(result_stmt);
-		result_stmt = vy_stmt_dup(old_stmt, old_type == IPROTO_UPSERT ?
-						    upsert_format : format);
+		result_stmt = vy_stmt_dup(old_stmt, format);
 	}
 	return result_stmt;
 }
diff --git a/src/box/vy_upsert.h b/src/box/vy_upsert.h
index f19228a1..7878b1b1 100644
--- a/src/box/vy_upsert.h
+++ b/src/box/vy_upsert.h
@@ -58,7 +58,6 @@ struct tuple_format;
  * @param old_stmt       An REPLACE/DELETE/UPSERT statement or NULL.
  * @param cmp_def        Key definition of an index, with primary parts.
  * @param format         Format for REPLACE/DELETE tuples.
- * @param upsert_format  Format for UPSERT tuples.
  * @param suppress_error True if ClientErrors must not be written to log.
  *
  * @retval NULL     Memory allocation error.
@@ -67,7 +66,7 @@ struct tuple_format;
 struct tuple *
 vy_apply_upsert(const struct tuple *new_stmt, const struct tuple *old_stmt,
 		const struct key_def *cmp_def, struct tuple_format *format,
-		struct tuple_format *upsert_format, bool suppress_error);
+		bool suppress_error);
 
 #if defined(__cplusplus)
 } /* extern "C" */
diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
index c2023c2c..52b28aca 100644
--- a/src/box/vy_write_iterator.c
+++ b/src/box/vy_write_iterator.c
@@ -171,8 +171,6 @@ struct vy_write_iterator {
 	const struct key_def *cmp_def;
 	/** Format to allocate new REPLACE and DELETE tuples from vy_run */
 	struct tuple_format *format;
-	/** Same as format, but for UPSERT tuples. */
-	struct tuple_format *upsert_format;
 	/* There is no LSM tree level older than the one we're writing to. */
 	bool is_last_level;
 	/**
@@ -330,9 +328,10 @@ static const struct vy_stmt_stream_iface vy_slice_stream_iface;
  * @return the iterator or NULL on error (diag is set).
  */
 struct vy_stmt_stream *
-vy_write_iterator_new(const struct key_def *cmp_def, struct tuple_format *format,
-		      struct tuple_format *upsert_format, bool is_primary,
-		      bool is_last_level, struct rlist *read_views)
+vy_write_iterator_new(const struct key_def *cmp_def,
+		      struct tuple_format *format,
+		      bool is_primary, bool is_last_level,
+		      struct rlist *read_views)
 {
 	/*
 	 * One is reserved for INT64_MAX - maximal read view.
@@ -365,8 +364,6 @@ vy_write_iterator_new(const struct key_def *cmp_def, struct tuple_format *format
 	stream->cmp_def = cmp_def;
 	stream->format = format;
 	tuple_format_ref(stream->format);
-	stream->upsert_format = upsert_format;
-	tuple_format_ref(stream->upsert_format);
 	stream->is_primary = is_primary;
 	stream->is_last_level = is_last_level;
 	return &stream->base;
@@ -415,7 +412,6 @@ vy_write_iterator_close(struct vy_stmt_stream *vstream)
 	struct vy_write_iterator *stream = (struct vy_write_iterator *)vstream;
 	vy_write_iterator_stop(vstream);
 	tuple_format_unref(stream->format);
-	tuple_format_unref(stream->upsert_format);
 	free(stream);
 }
 
@@ -447,8 +443,7 @@ vy_write_iterator_new_slice(struct vy_stmt_stream *vstream,
 	if (src == NULL)
 		return -1;
 	vy_slice_stream_open(&src->slice_stream, slice, stream->cmp_def,
-			     stream->format, stream->upsert_format,
-			     stream->is_primary);
+			     stream->format, stream->is_primary);
 	return 0;
 }
 
@@ -767,10 +762,8 @@ vy_read_view_merge(struct vy_write_iterator *stream, struct tuple *hint,
 	     vy_stmt_type(hint) != IPROTO_UPSERT))) {
 		assert(!stream->is_last_level || hint == NULL ||
 		       vy_stmt_type(hint) != IPROTO_UPSERT);
-		struct tuple *applied =
-			vy_apply_upsert(h->tuple, hint,
-					stream->cmp_def, stream->format,
-					stream->upsert_format, false);
+		struct tuple *applied = vy_apply_upsert(h->tuple, hint,
+				stream->cmp_def, stream->format, false);
 		if (applied == NULL)
 			return -1;
 		vy_stmt_unref_if_possible(h->tuple);
@@ -783,10 +776,8 @@ vy_read_view_merge(struct vy_write_iterator *stream, struct tuple *hint,
 		assert(h->tuple != NULL &&
 		       vy_stmt_type(h->tuple) == IPROTO_UPSERT);
 		assert(result->tuple != NULL);
-		struct tuple *applied =
-			vy_apply_upsert(h->tuple, result->tuple,
-					stream->cmp_def, stream->format,
-					stream->upsert_format, false);
+		struct tuple *applied = vy_apply_upsert(h->tuple, result->tuple,
+					stream->cmp_def, stream->format, false);
 		if (applied == NULL)
 			return -1;
 		vy_stmt_unref_if_possible(result->tuple);
diff --git a/src/box/vy_write_iterator.h b/src/box/vy_write_iterator.h
index fd8f214d..ea14b07a 100644
--- a/src/box/vy_write_iterator.h
+++ b/src/box/vy_write_iterator.h
@@ -224,16 +224,16 @@ struct vy_slice;
  * use vy_write_iterator_add_* functions.
  * @param cmp_def - key definition for tuple compare.
  * @param format - dormat to allocate new REPLACE and DELETE tuples from vy_run.
- * @param upsert_format - same as format, but for UPSERT tuples.
  * @param LSM tree is_primary - set if this iterator is for a primary index.
  * @param is_last_level - there is no older level than the one we're writing to.
  * @param read_views - Opened read views.
  * @return the iterator or NULL on error (diag is set).
  */
 struct vy_stmt_stream *
-vy_write_iterator_new(const struct key_def *cmp_def, struct tuple_format *format,
-		      struct tuple_format *upsert_format, bool is_primary,
-		      bool is_last_level, struct rlist *read_views);
+vy_write_iterator_new(const struct key_def *cmp_def,
+		      struct tuple_format *format,
+		      bool is_primary, bool is_last_level,
+		      struct rlist *read_views);
 
 /**
  * Add a mem as a source to the iterator.
diff --git a/test/unit/vy_cache.c b/test/unit/vy_cache.c
index 37d98828..6b543d85 100644
--- a/test/unit/vy_cache.c
+++ b/test/unit/vy_cache.c
@@ -16,8 +16,8 @@ test_basic()
 	struct tuple_format *format;
 	create_test_cache(fields, types, lengthof(fields), &cache, &key_def,
 			  &format);
-	struct tuple *select_all =
-		vy_new_simple_stmt(format, NULL, NULL, &key_template);
+	struct tuple *select_all = vy_new_simple_stmt(format, NULL,
+						      &key_template);
 
 	/*
 	 * Fill the cache with 3 chains.
@@ -87,7 +87,7 @@ test_basic()
 	bool unused;
 	for (int i = 0; i < 4; ++i)
 		vy_cache_iterator_next(&itr, &ret, &unused);
-	ok(vy_stmt_are_same(ret, &chain1[3], format, NULL, NULL),
+	ok(vy_stmt_are_same(ret, &chain1[3], format, NULL),
 	   "next_key * 4");
 
 	/*
@@ -106,11 +106,10 @@ test_basic()
 	 * the last_stmt. So restore on chain1[0], but the result
 	 * must be chain1[1].
 	 */
-	struct tuple *last_stmt =
-		vy_new_simple_stmt(format, NULL, NULL, &chain1[0]);
+	struct tuple *last_stmt = vy_new_simple_stmt(format, NULL, &chain1[0]);
 	ok(vy_cache_iterator_restore(&itr, last_stmt, &ret, &unused) >= 0,
 	   "restore");
-	ok(vy_stmt_are_same(ret, &chain1[1], format, NULL, NULL),
+	ok(vy_stmt_are_same(ret, &chain1[1], format, NULL),
 	   "restore on position after last");
 	tuple_unref(last_stmt);
 
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index 9eab13d3..642d8bf2 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -42,7 +42,6 @@ vy_iterator_C_test_finish()
 
 struct tuple *
 vy_new_simple_stmt(struct tuple_format *format,
-		   struct tuple_format *upsert_format,
 		   struct tuple_format *format_with_colmask,
 		   const struct vy_stmt_template *templ)
 {
@@ -121,8 +120,7 @@ vy_new_simple_stmt(struct tuple_format *format,
 		operations[0].iov_base = tmp;
 		operations[0].iov_len = ops - tmp;
 		fail_if(templ->optimize_update);
-		ret = vy_stmt_new_upsert(upsert_format, buf, pos,
-					 operations, 1);
+		ret = vy_stmt_new_upsert(format, buf, pos, operations, 1);
 		fail_if(ret == NULL);
 		break;
 	}
@@ -146,9 +144,8 @@ vy_new_simple_stmt(struct tuple_format *format,
 const struct tuple *
 vy_mem_insert_template(struct vy_mem *mem, const struct vy_stmt_template *templ)
 {
-	struct tuple *stmt =
-		vy_new_simple_stmt(mem->format, mem->upsert_format,
-				   mem->format_with_colmask, templ);
+	struct tuple *stmt = vy_new_simple_stmt(mem->format,
+			mem->format_with_colmask, templ);
 	struct tuple *region_stmt = vy_stmt_dup_lsregion(stmt,
 			&mem->env->allocator, mem->generation);
 	assert(region_stmt != NULL);
@@ -168,13 +165,12 @@ vy_cache_insert_templates_chain(struct vy_cache *cache,
 				const struct vy_stmt_template *key_templ,
 				enum iterator_type order)
 {
-	struct tuple *key =
-		vy_new_simple_stmt(format, NULL, NULL, key_templ);
+	struct tuple *key = vy_new_simple_stmt(format, NULL, key_templ);
 	struct tuple *prev_stmt = NULL;
 	struct tuple *stmt = NULL;
 
 	for (uint i = 0; i < length; ++i) {
-		stmt = vy_new_simple_stmt(format, NULL, NULL, &chain[i]);
+		stmt = vy_new_simple_stmt(format, NULL, &chain[i]);
 		vy_cache_add(cache, stmt, prev_stmt, key, order);
 		if (i != 0)
 			tuple_unref(prev_stmt);
@@ -190,7 +186,7 @@ void
 vy_cache_on_write_template(struct vy_cache *cache, struct tuple_format *format,
 			   const struct vy_stmt_template *templ)
 {
-	struct tuple *written = vy_new_simple_stmt(format, NULL, NULL, templ);
+	struct tuple *written = vy_new_simple_stmt(format, NULL, templ);
 	vy_cache_on_write(cache, written, NULL);
 	tuple_unref(written);
 }
@@ -221,14 +217,9 @@ create_test_mem(struct key_def *def)
 		vy_tuple_format_new_with_colmask(format);
 	assert(format_with_colmask != NULL);
 
-	/* Create upsert format */
-	struct tuple_format *format_upsert =
-		vy_tuple_format_new_upsert(format);
-	assert(format_upsert != NULL);
-
 	/* Create mem */
 	struct vy_mem *mem = vy_mem_new(&mem_env, 1, def, format,
-					format_with_colmask, format_upsert, 0);
+					format_with_colmask, 0);
 	fail_if(mem == NULL);
 	return mem;
 }
@@ -259,13 +250,12 @@ bool
 vy_stmt_are_same(const struct tuple *actual,
 		 const struct vy_stmt_template *expected,
 		 struct tuple_format *format,
-		 struct tuple_format *upsert_format,
 		 struct tuple_format *format_with_colmask)
 {
 	if (vy_stmt_type(actual) != expected->type)
 		return false;
-	struct tuple *tmp = vy_new_simple_stmt(format, upsert_format,
-					       format_with_colmask, expected);
+	struct tuple *tmp = vy_new_simple_stmt(format, format_with_colmask,
+					       expected);
 	fail_if(tmp == NULL);
 	uint32_t a_len, b_len;
 	const char *a, *b;
diff --git a/test/unit/vy_iterators_helper.h b/test/unit/vy_iterators_helper.h
index c5cbf982..e38ec295 100644
--- a/test/unit/vy_iterators_helper.h
+++ b/test/unit/vy_iterators_helper.h
@@ -95,7 +95,6 @@ struct vy_stmt_template {
  * Create a new vinyl statement using the specified template.
  *
  * @param format
- * @param upsert_format Format for upsert statements.
  * @param format_with_colmask Format for statements with a
  *        colmask.
  * @param templ Statement template.
@@ -104,7 +103,6 @@ struct vy_stmt_template {
  */
 struct tuple *
 vy_new_simple_stmt(struct tuple_format *format,
-		   struct tuple_format *upsert_format,
 		   struct tuple_format *format_with_colmask,
 		   const struct vy_stmt_template *templ);
 
@@ -204,7 +202,6 @@ destroy_test_cache(struct vy_cache *cache, struct key_def *def,
  * @param stmt Actual value.
  * @param templ Expected value.
  * @param format Template statement format.
- * @param upsert_format Template upsert statement format.
  * @param format_with_colmask Template statement format with colmask.
  *
  * @retval stmt === template.
@@ -213,7 +210,6 @@ bool
 vy_stmt_are_same(const struct tuple *actual,
 		 const struct vy_stmt_template *expected,
 		 struct tuple_format *format,
-		 struct tuple_format *upsert_format,
 		 struct tuple_format *format_with_colmask);
 
 #if defined(__cplusplus)
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index 6a3802de..bb9e4300 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -178,8 +178,7 @@ test_basic()
 	struct vy_mem *run_mem =
 		vy_mem_new(pk->mem->env, *pk->env->p_generation,
 			   pk->cmp_def, pk->mem_format,
-			   pk->mem_format_with_colmask,
-			   pk->upsert_format, 0);
+			   pk->mem_format_with_colmask, 0);
 
 	for (size_t i = 0; i < num_of_keys; i++) {
 		if (!in_run2[i])
@@ -192,8 +191,7 @@ test_basic()
 	}
 	struct vy_stmt_stream *write_stream
 		= vy_write_iterator_new(pk->cmp_def, pk->disk_format,
-					pk->upsert_format, true,
-					true, &read_views);
+					true, true, &read_views);
 	vy_write_iterator_new_mem(write_stream, run_mem);
 	struct vy_run *run = vy_run_new(&run_env, 1);
 	isnt(run, NULL, "vy_run_new");
@@ -213,8 +211,7 @@ test_basic()
 	run_mem =
 		vy_mem_new(pk->mem->env, *pk->env->p_generation,
 			   pk->cmp_def, pk->mem_format,
-			   pk->mem_format_with_colmask,
-			   pk->upsert_format, 0);
+			   pk->mem_format_with_colmask, 0);
 
 	for (size_t i = 0; i < num_of_keys; i++) {
 		if (!in_run1[i])
@@ -227,8 +224,7 @@ test_basic()
 	}
 	write_stream
 		= vy_write_iterator_new(pk->cmp_def, pk->disk_format,
-					pk->upsert_format, true,
-					true, &read_views);
+					true, true, &read_views);
 	vy_write_iterator_new_mem(write_stream, run_mem);
 	run = vy_run_new(&run_env, 2);
 	isnt(run, NULL, "vy_run_new");
@@ -274,10 +270,8 @@ test_basic()
 
 			struct vy_stmt_template tmpl_key =
 				STMT_TEMPLATE(0, SELECT, i);
-			struct tuple *key =
-				vy_new_simple_stmt(format, pk->upsert_format,
-						   pk->mem_format_with_colmask,
-						   &tmpl_key);
+			struct tuple *key = vy_new_simple_stmt(format,
+					pk->mem_format_with_colmask, &tmpl_key);
 			struct tuple *res;
 			rc = vy_point_lookup(pk, NULL, &prv, key, &res);
 			tuple_unref(key);
diff --git a/test/unit/vy_write_iterator.c b/test/unit/vy_write_iterator.c
index bf8ef39e..6a112028 100644
--- a/test/unit/vy_write_iterator.c
+++ b/test/unit/vy_write_iterator.c
@@ -36,9 +36,8 @@ compare_write_iterator_results(struct key_def *key_def,
 	fail_if(rv_array == NULL);
 	init_read_views_list(&rv_list, rv_array, vlsns, vlsns_count);
 
-	struct vy_stmt_stream *wi =
-		vy_write_iterator_new(key_def, mem->format, mem->upsert_format,
-				      is_primary, is_last_level, &rv_list);
+	struct vy_stmt_stream *wi = vy_write_iterator_new(key_def, mem->format,
+					is_primary, is_last_level, &rv_list);
 	fail_if(wi == NULL);
 	fail_if(vy_write_iterator_new_mem(wi, mem) != 0);
 
@@ -51,7 +50,6 @@ compare_write_iterator_results(struct key_def *key_def,
 			break;
 		fail_if(i >= expected_count);
 		ok(vy_stmt_are_same(ret, &expected[i], mem->format,
-				    mem->upsert_format,
 				    mem->format_with_colmask),
 		   "stmt %d is correct", i);
 		++i;
-- 
2.11.0

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

* [tarantool-patches] Re: [PATCH 1/2] vinyl: allocate upsert counter on lsregion
  2018-03-30 15:47 ` [PATCH 1/2] vinyl: allocate upsert counter on lsregion Vladimir Davydov
@ 2018-04-02 10:27   ` v.shpilevoy
  2018-04-02 10:50     ` Vladimir Davydov
  0 siblings, 1 reply; 7+ messages in thread
From: v.shpilevoy @ 2018-04-02 10:27 UTC (permalink / raw)
  To: tarantool-patches; +Cc: kostja

Hello. Please, see 2 comments below.

> 30 марта 2018 г., в 18:47, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> Currently, we store upsert counter in tuple metadata (that's what
> upsert_format is for), but since it's only relevant for tuples of
> the memory level, we can store it on lsregion, right before tuple
> data. Let's do it now so that we can get rid of upsert_format.
> ---
> src/box/vy_stmt.c | 24 ++++++++++++++++--------
> src/box/vy_stmt.h | 23 ++++++++++++++---------
> 2 files changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
> index 84182e76..ac60c89e 100644
> --- a/src/box/vy_stmt.c
> +++ b/src/box/vy_stmt.c
> @@ -134,13 +134,26 @@ struct tuple *
> vy_stmt_dup_lsregion(const struct tuple *stmt, struct lsregion *lsregion,
> 		     int64_t alloc_id)
> {
> +	enum iproto_type type = vy_stmt_type(stmt);
> 	size_t size = tuple_size(stmt);
> +	size_t alloc_size = size;
> 	struct tuple *mem_stmt;
> -	mem_stmt = lsregion_alloc(lsregion, size, alloc_id);
> +
> +	/* Reserve one byte for UPSERT counter. */
> +	if (type == IPROTO_UPSERT)
> +		alloc_size++;
> +
> +	mem_stmt = lsregion_alloc(lsregion, alloc_size, alloc_id);
> 	if (mem_stmt == NULL) {
> 		diag_set(OutOfMemory, size, "lsregion_alloc", "mem_stmt");
> 		return NULL;
> 	}
> +
> +	if (type == IPROTO_UPSERT) {
> +		*(uint8_t *)mem_stmt = 0;

1. How about to at first, set mem_stmt and at second, call vy_stmt_set_n_upserts? I think,
that this pointers magic must be hidden where possible.

> +		mem_stmt = (struct tuple *)((uint8_t *)mem_stmt + 1);
> +	}
> +
> 	memcpy(mem_stmt, stmt, size);
> 	/*
> 	 * Region allocated statements can't be referenced or unreferenced
> 
> struct tuple *
> diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
> index a33739d6..8958adb5 100644
> --- a/src/box/vy_stmt.h
> +++ b/src/box/vy_stmt.h
> @@ -146,23 +146,28 @@ vy_stmt_set_type(struct tuple *stmt, enum iproto_type type)
> 	((struct vy_stmt *) stmt)->type = type;
> }
> 
> -/** Get upserts count of the vinyl statement. */
> +/**
> + * Get upserts count of the vinyl statement.
> + * Only for UPSERT statements allocated on lsregion.

2. Can you, please, add an assertion for this? For example:
assert(! vy_stmt_is_refable(stmt)).

> + */
> static inline uint8_t
> vy_stmt_n_upserts(const struct tuple *stmt)
> {
> -	assert(tuple_format(stmt)->extra_size == sizeof(uint8_t));
> -	return *((const uint8_t *) tuple_extra(stmt));
> +	assert(stmt->refs == 0);
> +	assert(vy_stmt_type(stmt) == IPROTO_UPSERT);
> +	return *((uint8_t *)stmt - 1);
> }
> 
> -/** Set upserts count of the vinyl statement. */
> +/**
> + * Set upserts count of the vinyl statement.
> + * Only for UPSERT statements allocated on lsregion.
> + */
> static inline void
> vy_stmt_set_n_upserts(struct tuple *stmt, uint8_t n)
> {
> -	struct tuple_format *format = tuple_format(stmt);
> -	assert(format->extra_size == sizeof(uint8_t));
> -	char *extra = (char *) stmt + stmt->data_offset -
> -		      tuple_format_meta_size(format);
> -	*((uint8_t *) extra) = n;
> +	assert(stmt->refs == 0);
> +	assert(vy_stmt_type(stmt) == IPROTO_UPSERT);
> +	*((uint8_t *)stmt - 1) = n;
> }
> 
> /** Get the column mask of the specified tuple. */
> -- 
> 2.11.0
> 
> 

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

* Re: [tarantool-patches] Re: [PATCH 1/2] vinyl: allocate upsert counter on lsregion
  2018-04-02 10:27   ` [tarantool-patches] " v.shpilevoy
@ 2018-04-02 10:50     ` Vladimir Davydov
  0 siblings, 0 replies; 7+ messages in thread
From: Vladimir Davydov @ 2018-04-02 10:50 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches, kostja

On Mon, Apr 02, 2018 at 01:27:14PM +0300, v.shpilevoy@tarantool.org wrote:
> > @@ -134,13 +134,26 @@ struct tuple *
> > vy_stmt_dup_lsregion(const struct tuple *stmt, struct lsregion *lsregion,
> > 		     int64_t alloc_id)
> > {
> > +	enum iproto_type type = vy_stmt_type(stmt);
> > 	size_t size = tuple_size(stmt);
> > +	size_t alloc_size = size;
> > 	struct tuple *mem_stmt;
> > -	mem_stmt = lsregion_alloc(lsregion, size, alloc_id);
> > +
> > +	/* Reserve one byte for UPSERT counter. */
> > +	if (type == IPROTO_UPSERT)
> > +		alloc_size++;
> > +
> > +	mem_stmt = lsregion_alloc(lsregion, alloc_size, alloc_id);
> > 	if (mem_stmt == NULL) {
> > 		diag_set(OutOfMemory, size, "lsregion_alloc", "mem_stmt");
> > 		return NULL;
> > 	}
> > +
> > +	if (type == IPROTO_UPSERT) {
> > +		*(uint8_t *)mem_stmt = 0;
> 
> 1. How about to at first, set mem_stmt and at second, call vy_stmt_set_n_upserts? I think,
> that this pointers magic must be hidden where possible.

I thought about that, but I'd have to play with pointers anyway - see
right below how I adjust mem_stmt address.

> > +		mem_stmt = (struct tuple *)((uint8_t *)mem_stmt + 1);

This has to be done irrespective of whether I use vy_stmt_set_n_upserts
or not. And since I already play with pointers, I reckoned that it would
be OK to assign n_upserts here as well rather than scatter the logic
between two functions.

> > +	}
> > +
> > 	memcpy(mem_stmt, stmt, size);
> > 	/*
> > 	 * Region allocated statements can't be referenced or unreferenced
> > 
> > struct tuple *
> > diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
> > index a33739d6..8958adb5 100644
> > --- a/src/box/vy_stmt.h
> > +++ b/src/box/vy_stmt.h
> > @@ -146,23 +146,28 @@ vy_stmt_set_type(struct tuple *stmt, enum iproto_type type)
> > 	((struct vy_stmt *) stmt)->type = type;
> > }
> > 
> > -/** Get upserts count of the vinyl statement. */
> > +/**
> > + * Get upserts count of the vinyl statement.
> > + * Only for UPSERT statements allocated on lsregion.
> 
> 2. Can you, please, add an assertion for this? For example:
> assert(! vy_stmt_is_refable(stmt)).

I did: see 'assert(stmt->refs == 0)' right below.

I didn't use vy_stmt_is_refable(), because it's defined below this
function, and I didn't want to move it.

> 
> > + */
> > static inline uint8_t
> > vy_stmt_n_upserts(const struct tuple *stmt)
> > {
> > -	assert(tuple_format(stmt)->extra_size == sizeof(uint8_t));
> > -	return *((const uint8_t *) tuple_extra(stmt));
> > +	assert(stmt->refs == 0);
> > +	assert(vy_stmt_type(stmt) == IPROTO_UPSERT);
> > +	return *((uint8_t *)stmt - 1);
> > }

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

* Re: [PATCH 2/2] vinyl: zap upsert_format
  2018-03-30 15:47 ` [PATCH 2/2] vinyl: zap upsert_format Vladimir Davydov
@ 2018-04-02 10:58   ` v.shpilevoy
  2018-04-02 11:06     ` Vladimir Davydov
  0 siblings, 1 reply; 7+ messages in thread
From: v.shpilevoy @ 2018-04-02 10:58 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: Konstantin Osipov, tarantool-patches

Please, see 2 comments below.

> 30 марта 2018 г., в 18:47, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> The only difference between format of UPSERT statements and format of
> other DML statements of the same index is that the former reserves one
> byte for UPSERT counter, which is needed to schedule UPSERT squashing.
> Since we store UPSERT counter on lsregion now, we don't need a special
> format for UPSERTs anymore. Remove it.
> ---
> src/box/vinyl.c                 | 33 ++++------------------------
> src/box/vy_lsm.c                | 48 +++++++++++------------------------------
> src/box/vy_lsm.h                |  5 -----
> src/box/vy_mem.c                | 15 ++++---------
> src/box/vy_mem.h                |  6 +-----
> src/box/vy_point_lookup.c       |  6 ++----
> src/box/vy_read_iterator.c      |  5 ++---
> src/box/vy_run.c                | 33 +++++++++++-----------------
> src/box/vy_run.h                | 11 ++--------
> src/box/vy_scheduler.c          |  8 +++----
> src/box/vy_stmt.c               | 48 +++++------------------------------------
> src/box/vy_stmt.h               | 26 +---------------------
> src/box/vy_tx.c                 |  9 +++-----
> src/box/vy_upsert.c             | 11 +++++-----
> src/box/vy_upsert.h             |  3 +--
> src/box/vy_write_iterator.c     | 27 ++++++++---------------
> src/box/vy_write_iterator.h     |  8 +++----
> test/unit/vy_cache.c            | 11 +++++-----
> test/unit/vy_iterators_helper.c | 28 ++++++++----------------
> test/unit/vy_iterators_helper.h |  4 ----
> test/unit/vy_point_lookup.c     | 18 ++++++----------
> test/unit/vy_write_iterator.c   |  6 ++----
> 22 files changed, 95 insertions(+), 274 deletions(-)
> 
> diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
> index 1105340c..68abf5bc 100644
> --- a/src/box/vy_mem.c
> +++ b/src/box/vy_mem.c
> @@ -128,19 +125,15 @@ vy_mem_new(struct vy_mem_env *env, int64_t generation,
> 

1. This functions seems to be unused. Can you delete it, please?

> void
> vy_mem_update_formats(struct vy_mem *mem, struct tuple_format *new_format,
> -		      struct tuple_format *new_format_with_colmask,
> -		      struct tuple_format *new_upsert_format)
> +		      struct tuple_format *new_format_with_colmask)
> {
> 	assert(mem->count.rows == 0);
> 	tuple_format_unref(mem->format);
> 	tuple_format_unref(mem->format_with_colmask);
> -	tuple_format_unref(mem->upsert_format);
> 	mem->format = new_format;
> 	mem->format_with_colmask = new_format_with_colmask;
> -	mem->upsert_format = new_upsert_format;
> 	tuple_format_ref(mem->format);
> 	tuple_format_ref(mem->format_with_colmask);
> -	tuple_format_ref(mem->upsert_format);
> }
> 
> void
> diff --git a/src/box/vy_run.c b/src/box/vy_run.c
> index da6d5872..637f63fa 100644
> --- a/src/box/vy_run.c
> +++ b/src/box/vy_run.c
> @@ -2205,7 +2202,6 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,

2. Please, update a comment - it seems to be very out of date: user_key_def is omitted,
and bloom_fpr is already removed.

> 		     const struct key_def *cmp_def,
> 		     const struct key_def *key_def,
> 		     struct tuple_format *mem_format,
> -		     struct tuple_format *upsert_format,
> 		     const struct index_opts *opts)
> {
> 	assert(run->info.bloom == NULL);
> 
> 

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

* Re: [PATCH 2/2] vinyl: zap upsert_format
  2018-04-02 10:58   ` v.shpilevoy
@ 2018-04-02 11:06     ` Vladimir Davydov
  0 siblings, 0 replies; 7+ messages in thread
From: Vladimir Davydov @ 2018-04-02 11:06 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: Konstantin Osipov, tarantool-patches

On Mon, Apr 02, 2018 at 01:58:36PM +0300, v.shpilevoy@tarantool.org wrote:
> 1. This functions seems to be unused. Can you delete it, please?

Already did, in a separate patch:

  https://www.freelists.org/post/tarantool-patches/PATCH-0712-vinyl-zap-vy-mem-update-formats

> > void
> > vy_mem_update_formats(struct vy_mem *mem, struct tuple_format *new_format,
> > -		      struct tuple_format *new_format_with_colmask,
> > -		      struct tuple_format *new_upsert_format)
> > +		      struct tuple_format *new_format_with_colmask)
> > {
> > 	assert(mem->count.rows == 0);
> > 	tuple_format_unref(mem->format);
> > 	tuple_format_unref(mem->format_with_colmask);
> > -	tuple_format_unref(mem->upsert_format);
> > 	mem->format = new_format;
> > 	mem->format_with_colmask = new_format_with_colmask;
> > -	mem->upsert_format = new_upsert_format;
> > 	tuple_format_ref(mem->format);
> > 	tuple_format_ref(mem->format_with_colmask);
> > -	tuple_format_ref(mem->upsert_format);
> > }
> > 
> > void
> > diff --git a/src/box/vy_run.c b/src/box/vy_run.c
> > index da6d5872..637f63fa 100644
> > --- a/src/box/vy_run.c
> > +++ b/src/box/vy_run.c
> > @@ -2205,7 +2202,6 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
> 
> 2. Please, update a comment - it seems to be very out of date: user_key_def is omitted,
> and bloom_fpr is already removed.

OK. And it looks like this function should use disk_format instead of
mem_format. I'll update it in a separate patch.

> 
> > 		     const struct key_def *cmp_def,
> > 		     const struct key_def *key_def,
> > 		     struct tuple_format *mem_format,
> > -		     struct tuple_format *upsert_format,
> > 		     const struct index_opts *opts)
> > {
> > 	assert(run->info.bloom == NULL);

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

end of thread, other threads:[~2018-04-02 11:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 15:47 [PATCH 0/2] vinyl: zap upsert_format Vladimir Davydov
2018-03-30 15:47 ` [PATCH 1/2] vinyl: allocate upsert counter on lsregion Vladimir Davydov
2018-04-02 10:27   ` [tarantool-patches] " v.shpilevoy
2018-04-02 10:50     ` Vladimir Davydov
2018-03-30 15:47 ` [PATCH 2/2] vinyl: zap upsert_format Vladimir Davydov
2018-04-02 10:58   ` v.shpilevoy
2018-04-02 11:06     ` Vladimir Davydov

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