From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 1.10 1/2] vinyl: clean up region after allocating surrogate statement Date: Tue, 25 Jun 2019 15:46:04 +0300 Message-Id: <0e7c933dc549fea020465dbf09c8e08fb2b3bc45.1561466574.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: tarantool-patches@freelists.org List-ID: vy_stmt_new_surrogate_from_key() and vy_stmt_new_surrogate_delete_raw() allocate temporary objects on the region, but don't clean up after themselves. Those functions may be called by a vinyl reader threads: vy_page_read_cb vy_page_find_key vy_page_stmt vy_stmt_decode In this case the region will grow infinitely, because reader threads never call fiber_gc(). The leak was introduced to 1.10 by commit b907231713a7 ("vinyl: lookup key in reader thread"), which moved vy_page_find_key() invocation to reader threads for the sake of performance. The fix is trivial - call region_truncate() from those functions. Note, neither the master branch nor 2.1 is affected to this issue, because region_truncate() was added there in the scope of the multikey index feature. --- src/box/vy_stmt.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c index ca4c55f4..9b7c5551 100644 --- a/src/box/vy_stmt.c +++ b/src/box/vy_stmt.c @@ -383,6 +383,7 @@ vy_stmt_new_surrogate_from_key(const char *key, enum iproto_type type, /* UPSERT can't be surrogate. */ assert(type != IPROTO_UPSERT); struct region *region = &fiber()->gc; + size_t region_svp = region_used(region); uint32_t field_count = format->index_field_count; struct iovec *iov = region_alloc(region, sizeof(*iov) * field_count); @@ -410,7 +411,7 @@ vy_stmt_new_surrogate_from_key(const char *key, enum iproto_type type, struct tuple *stmt = vy_stmt_alloc(format, bsize); if (stmt == NULL) - return NULL; + goto out; char *raw = (char *) tuple_data(stmt); uint32_t *field_map = (uint32_t *) raw; @@ -428,6 +429,8 @@ vy_stmt_new_surrogate_from_key(const char *key, enum iproto_type type, } assert(wpos == raw + bsize); vy_stmt_set_type(stmt, type); +out: + region_truncate(region, region_svp); return stmt; } @@ -443,10 +446,13 @@ struct tuple * vy_stmt_new_surrogate_delete_raw(struct tuple_format *format, const char *src_data, const char *src_data_end) { + struct region *region = &fiber()->gc; + size_t region_svp = region_used(region); + uint32_t src_size = src_data_end - src_data; uint32_t total_size = src_size + format->field_map_size; /* Surrogate tuple uses less memory than the original tuple */ - char *data = region_alloc(&fiber()->gc, total_size); + char *data = region_alloc(region, total_size); if (data == NULL) { diag_set(OutOfMemory, src_size, "region", "tuple"); return NULL; @@ -490,13 +496,14 @@ vy_stmt_new_surrogate_delete_raw(struct tuple_format *format, uint32_t bsize = pos - data; struct tuple *stmt = vy_stmt_alloc(format, bsize); if (stmt == NULL) - return NULL; + goto out; char *stmt_data = (char *) tuple_data(stmt); char *stmt_field_map_begin = stmt_data - format->field_map_size; memcpy(stmt_data, data, bsize); memcpy(stmt_field_map_begin, field_map_begin, format->field_map_size); vy_stmt_set_type(stmt, IPROTO_DELETE); - +out: + region_truncate(region, region_svp); return stmt; } -- 2.11.0