From: Vladimir Davydov <vdavydov.dev@gmail.com> To: tarantool-patches@freelists.org Subject: [PATCH 1.10 1/2] vinyl: clean up region after allocating surrogate statement Date: Tue, 25 Jun 2019 15:46:04 +0300 [thread overview] Message-ID: <0e7c933dc549fea020465dbf09c8e08fb2b3bc45.1561466574.git.vdavydov.dev@gmail.com> (raw) In-Reply-To: <cover.1561466574.git.vdavydov.dev@gmail.com> In-Reply-To: <cover.1561466574.git.vdavydov.dev@gmail.com> 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
next prev parent reply other threads:[~2019-06-25 12:46 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-25 12:46 [PATCH 1.10 0/2] Vinyl memory leak fixes Vladimir Davydov 2019-06-25 12:46 ` Vladimir Davydov [this message] 2019-06-25 12:46 ` [PATCH 1.10 2/2] vinyl: free region on vylog commit instead of resetting it Vladimir Davydov 2019-06-25 12:46 ` [PATCH 1.10 0/2] Vinyl memory leak fixes Vladimir Davydov
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=0e7c933dc549fea020465dbf09c8e08fb2b3bc45.1561466574.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 1.10 1/2] vinyl: clean up region after allocating surrogate statement' \ /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