Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Kosarev <i.kosarev@tarantool.org>
To: tarantool-patches@dev.tarantool.org
Cc: v.shpilevoy@tarantool.org
Subject: [Tarantool-patches] [PATCH 3/3] tuple: use calloc for service truncation tuples
Date: Fri, 13 Dec 2019 10:05:11 +0300	[thread overview]
Message-ID: <5fabea2d6e2100223f001b32d42c8c9630f33657.1575627361.git.i.kosarev@tarantool.org> (raw)
In-Reply-To: <cover.1575627361.git.i.kosarev@tarantool.org>
In-Reply-To: <cover.1575627361.git.i.kosarev@tarantool.org>

Trying to perform space:truncate() while reaching memtx_memory limit we
could experience slab allocator failure on allocation for service tuple
being upserted into service space to keep record of truncations. This
behavior seems to be quite surprising for users. Now we are using
calloc() to allocate tuples in BOX_TRUNCATE_ID space.

Closes: #3807
---
 src/box/memtx_engine.c | 50 ++++++++++++++++++++++++++++--------------
 src/box/memtx_engine.h |  2 +-
 src/box/memtx_space.c  | 20 ++++++++++++-----
 src/box/tuple.c        | 16 ++++++++++----
 src/box/tuple.h        |  2 +-
 src/box/tuple_format.c |  1 +
 src/box/tuple_format.h | 10 ++++++++-
 src/box/vy_stmt.c      |  3 ++-
 8 files changed, 74 insertions(+), 30 deletions(-)

diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index 23ccc4703fe..59148059f2b 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -1113,7 +1113,7 @@ memtx_leave_delayed_free_mode(struct memtx_engine *memtx)
 }
 
 struct tuple *
-memtx_tuple_new(struct tuple_format *format, const char *data, const char *end)
+memtx_tuple_new(struct tuple_format *format, const char *data, const char *end, bool use_calloc)
 {
 	struct memtx_engine *memtx = (struct memtx_engine *)format->engine;
 	assert(mp_typeof(*data) == MP_ARRAY);
@@ -1139,15 +1139,27 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end)
 	}
 
 	struct memtx_tuple *memtx_tuple;
-	while ((memtx_tuple = smalloc(&memtx->alloc, total)) == NULL) {
-		bool stop;
-		memtx_engine_run_gc(memtx, &stop);
-		if (stop)
-			break;
-	}
-	if (memtx_tuple == NULL) {
-		diag_set(OutOfMemory, total, "slab allocator", "memtx_tuple");
-		goto end;
+	if (use_calloc) {
+		memtx_tuple = calloc(1, total);
+		if (memtx_tuple == NULL) {
+			diag_set(OutOfMemory, total, "malloc",
+						     "memtx_tuple");
+			goto end;
+		}
+		format->is_malloced = true;
+	} else {
+		while ((memtx_tuple = smalloc(&memtx->alloc,
+					      total)) == NULL) {
+			bool stop;
+			memtx_engine_run_gc(memtx, &stop);
+			if (stop)
+				break;
+		}
+		if (memtx_tuple == NULL) {
+			diag_set(OutOfMemory, total, "slab allocator",
+						     "memtx_tuple");
+			goto end;
+		}
 	}
 	tuple = &memtx_tuple->base;
 	tuple->refs = 0;
@@ -1177,16 +1189,20 @@ memtx_tuple_delete(struct tuple_format *format, struct tuple *tuple)
 	struct memtx_engine *memtx = (struct memtx_engine *)format->engine;
 	say_debug("%s(%p)", __func__, tuple);
 	assert(tuple->refs == 0);
-	tuple_format_unref(format);
 	struct memtx_tuple *memtx_tuple =
 		container_of(tuple, struct memtx_tuple, base);
 	size_t total = tuple_size(tuple) + offsetof(struct memtx_tuple, base);
-	if (memtx->alloc.free_mode != SMALL_DELAYED_FREE ||
-	    memtx_tuple->version == memtx->snapshot_version ||
-	    format->is_temporary)
-		smfree(&memtx->alloc, memtx_tuple, total);
-	else
-		smfree_delayed(&memtx->alloc, memtx_tuple, total);
+	if (format->is_malloced) {
+		free(memtx_tuple);
+	} else {
+		if (memtx->alloc.free_mode != SMALL_DELAYED_FREE ||
+		    memtx_tuple->version == memtx->snapshot_version ||
+		    format->is_temporary)
+			smfree(&memtx->alloc, memtx_tuple, total);
+		else
+			smfree_delayed(&memtx->alloc, memtx_tuple, total);
+	}
+	tuple_format_unref(format);
 }
 
 void
diff --git a/src/box/memtx_engine.h b/src/box/memtx_engine.h
index f562c66df4f..a2ae8134d30 100644
--- a/src/box/memtx_engine.h
+++ b/src/box/memtx_engine.h
@@ -235,7 +235,7 @@ memtx_leave_delayed_free_mode(struct memtx_engine *memtx);
 
 /** Allocate a memtx tuple. @sa tuple_new(). */
 struct tuple *
-memtx_tuple_new(struct tuple_format *format, const char *data, const char *end);
+memtx_tuple_new(struct tuple_format *format, const char *data, const char *end, bool use_calloc);
 
 /** Free a memtx tuple. @sa tuple_delete(). */
 void
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 1e8759778e7..c413fd9b1e3 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -332,7 +332,9 @@ memtx_space_execute_replace(struct space *space, struct txn *txn,
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	enum dup_replace_mode mode = dup_replace_mode(request->type);
 	stmt->new_tuple = memtx_tuple_new(space->format, request->tuple,
-					  request->tuple_end);
+					  request->tuple_end,
+					  request->space_id
+					  == BOX_TRUNCATE_ID);
 	if (stmt->new_tuple == NULL)
 		return -1;
 	tuple_ref(stmt->new_tuple);
@@ -406,7 +408,9 @@ memtx_space_execute_update(struct space *space, struct txn *txn,
 		return -1;
 
 	stmt->new_tuple = memtx_tuple_new(format, new_data,
-					  new_data + new_size);
+					  new_data + new_size,
+					  request->space_id
+					  == BOX_TRUNCATE_ID);
 	if (stmt->new_tuple == NULL)
 		return -1;
 	tuple_ref(stmt->new_tuple);
@@ -475,7 +479,9 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 			return -1;
 		}
 		stmt->new_tuple = memtx_tuple_new(format, request->tuple,
-						  request->tuple_end);
+						  request->tuple_end,
+						  request->space_id
+						  == BOX_TRUNCATE_ID);
 		if (stmt->new_tuple == NULL)
 			return -1;
 		tuple_ref(stmt->new_tuple);
@@ -499,7 +505,9 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
 			return -1;
 
 		stmt->new_tuple = memtx_tuple_new(format, new_data,
-						  new_data + new_size);
+						  new_data + new_size,
+						  request->space_id
+						  == BOX_TRUNCATE_ID);
 		if (stmt->new_tuple == NULL)
 			return -1;
 		tuple_ref(stmt->new_tuple);
@@ -548,7 +556,9 @@ memtx_space_ephemeral_replace(struct space *space, const char *tuple,
 {
 	struct memtx_space *memtx_space = (struct memtx_space *)space;
 	struct tuple *new_tuple = memtx_tuple_new(space->format, tuple,
-						  tuple_end);
+						  tuple_end,
+						  space->def->id
+						  == BOX_TRUNCATE_ID);
 	if (new_tuple == NULL)
 		return -1;
 	struct tuple *old_tuple;
diff --git a/src/box/tuple.c b/src/box/tuple.c
index 4d676b090c2..bafa8e0cb5c 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -60,7 +60,7 @@ static void
 runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple);
 
 static struct tuple *
-runtime_tuple_new(struct tuple_format *format, const char *data, const char *end);
+runtime_tuple_new(struct tuple_format *format, const char *data, const char *end, bool use_calloc);
 
 /** A virtual method table for tuple_format_runtime */
 static struct tuple_format_vtab tuple_format_runtime_vtab = {
@@ -71,7 +71,7 @@ static struct tuple_format_vtab tuple_format_runtime_vtab = {
 };
 
 static struct tuple *
-runtime_tuple_new(struct tuple_format *format, const char *data, const char *end)
+runtime_tuple_new(struct tuple_format *format, const char *data, const char *end, bool use_calloc)
 {
 	assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete);
 
@@ -86,8 +86,13 @@ runtime_tuple_new(struct tuple_format *format, const char *data, const char *end
 
 	size_t data_len = end - data;
 	size_t total = sizeof(struct tuple) + field_map_size + data_len;
-	tuple = (struct tuple *) smalloc(&runtime_alloc, total);
+	if (use_calloc) {
+		tuple = (struct tuple *) calloc(1, total);
+		format->is_malloced = true;
+	} else
+		tuple = (struct tuple *) smalloc(&runtime_alloc, total);
 	if (tuple == NULL) {
+		format->is_malloced = false;
 		diag_set(OutOfMemory, (unsigned) total,
 			 "malloc", "tuple");
 		goto end;
@@ -114,8 +119,11 @@ runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple)
 	say_debug("%s(%p)", __func__, tuple);
 	assert(tuple->refs == 0);
 	size_t total = tuple_size(tuple);
+	if (format->is_malloced)
+		free(tuple);
+	else
+		smfree(&runtime_alloc, tuple, total);
 	tuple_format_unref(format);
-	smfree(&runtime_alloc, tuple, total);
 }
 
 int
diff --git a/src/box/tuple.h b/src/box/tuple.h
index 9a887721986..7fa62659acc 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -431,7 +431,7 @@ tuple_format(struct tuple *tuple)
 static inline struct tuple *
 tuple_new(struct tuple_format *format, const char *data, const char *end)
 {
-	return format->vtab.tuple_new(format, data, end);
+	return format->vtab.tuple_new(format, data, end, false);
 }
 
 /**
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 312c96621fb..d0e60acd334 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -789,6 +789,7 @@ tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
 		memset(&format->vtab, 0, sizeof(format->vtab));
 	format->engine = engine;
 	format->is_temporary = is_temporary;
+	format->is_malloced = false;
 	format->is_ephemeral = is_ephemeral;
 	format->exact_field_count = exact_field_count;
 	format->epoch = ++formats_epoch;
diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
index 021072d3d4c..cb9f0a0407d 100644
--- a/src/box/tuple_format.h
+++ b/src/box/tuple_format.h
@@ -82,7 +82,7 @@ struct tuple_format_vtab {
 	 */
 	struct tuple*
 	(*tuple_new)(struct tuple_format *format, const char *data,
-	             const char *end);
+		     const char *end, bool use_calloc);
 	/**
 	 * Free a tuple_chunk allocated for given tuple and
 	 * data.
@@ -188,6 +188,14 @@ struct tuple_format {
 	 * in progress.
 	 */
 	bool is_temporary;
+	/**
+	 * Tuple is being malloced if we use calloc instead of
+	 * traditional smalloc to allocate it. It is happening
+	 * if the tuple is being upserted into BOX_TRUNCATE_ID
+	 * space as far as we want to perform space:truncate()
+	 * even if memtx_memory limit is reached.
+	 */
+	bool is_malloced;
 	/**
 	 * This format belongs to ephemeral space and thus might
 	 * be shared with other ephemeral spaces.
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 518a24f43d7..b6af0b00e5d 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -82,8 +82,9 @@ vy_stmt_persistent_flags(struct tuple *stmt, bool is_primary)
 }
 
 static struct tuple *
-vy_tuple_new(struct tuple_format *format, const char *data, const char *end)
+vy_tuple_new(struct tuple_format *format, const char *data, const char *end, bool use_calloc)
 {
+	(void)use_calloc;
 	if (tuple_validate_raw(format, data) != 0)
 		return NULL;
 
-- 
2.17.1

  parent reply	other threads:[~2019-12-13  7:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-13  7:05 [Tarantool-patches] [PATCH 0/3] Safe allocation on truncation and deletion Ilya Kosarev
2019-12-13  7:05 ` [Tarantool-patches] [PATCH 1/3] b-tree: return NULL on matras_alloc fail Ilya Kosarev
2019-12-19  0:32   ` Vladislav Shpilevoy
2019-12-13  7:05 ` [Tarantool-patches] [PATCH 2/3] memtx: don't reserve extra memory if not needed Ilya Kosarev
2019-12-19  0:31   ` Vladislav Shpilevoy
2019-12-13  7:05 ` Ilya Kosarev [this message]
2019-12-19  0:31   ` [Tarantool-patches] [PATCH 3/3] tuple: use calloc for service truncation tuples Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5fabea2d6e2100223f001b32d42c8c9630f33657.1575627361.git.i.kosarev@tarantool.org \
    --to=i.kosarev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 3/3] tuple: use calloc for service truncation tuples' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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