From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3EB134696C4 for ; Thu, 9 Apr 2020 00:37:11 +0300 (MSK) From: Nikita Pettik Date: Thu, 9 Apr 2020 00:37:07 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH 1/2] vinyl: init all vars before cleanup in vy_lsm_split_range() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org If vy_key_from_msgpack() fails in vy_lsm_split_range(), clean-up procedure is called. However, at this moment struct vy_range *parts[2] is not initialized ergo contains garbage and access to this structure may result in crash, segfault or disk formatting. Let's move initialization of mentioned variables before call of vy_lsm_split_range(). Part of #4864 --- src/box/vy_lsm.c | 4 +- src/box/vy_stmt.c | 5 + src/errinj.h | 1 + test/box/errinj.result | 1 + .../gh-4864-stmt-alloc-fail-compact.result | 93 +++++++++++++++++++ .../gh-4864-stmt-alloc-fail-compact.test.lua | 49 ++++++++++ test/vinyl/suite.ini | 2 +- 7 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 test/vinyl/gh-4864-stmt-alloc-fail-compact.result create mode 100644 test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index 3d3f41b7a..04c9926a8 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -1069,7 +1069,7 @@ vy_lsm_split_range(struct vy_lsm *lsm, struct vy_range *range) /* Split a range in two parts. */ const int n_parts = 2; - + struct vy_range *parts[2] = { NULL, NULL }; /* * Determine new ranges' boundaries. */ @@ -1088,7 +1088,7 @@ vy_lsm_split_range(struct vy_lsm *lsm, struct vy_range *range) * the old range's runs for them. */ struct vy_slice *slice, *new_slice; - struct vy_range *part, *parts[2] = {NULL, }; + struct vy_range *part = NULL; for (int i = 0; i < n_parts; i++) { part = vy_range_new(vy_log_next_id(), keys[i], keys[i + 1], lsm->cmp_def); diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c index 9b7c55516..d2469961f 100644 --- a/src/box/vy_stmt.c +++ b/src/box/vy_stmt.c @@ -134,6 +134,11 @@ vy_stmt_alloc(struct tuple_format *format, uint32_t bsize) { uint32_t total_size = sizeof(struct vy_stmt) + format->field_map_size + bsize; + struct errinj *inj = errinj(ERRINJ_VY_MAX_TUPLE_SIZE, ERRINJ_INT); + if (inj != NULL && inj->iparam >= 0) { + if (inj->iparam-- == 0) + total_size = vy_max_tuple_size + 1; + } if (unlikely(total_size > vy_max_tuple_size)) { diag_set(ClientError, ER_VINYL_MAX_TUPLE_SIZE, (unsigned) total_size); diff --git a/src/errinj.h b/src/errinj.h index 2cb090b68..dd81938e8 100644 --- a/src/errinj.h +++ b/src/errinj.h @@ -127,6 +127,7 @@ struct errinj { _(ERRINJ_VY_COMPACTION_DELAY, ERRINJ_BOOL, {.bparam = false}) \ _(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT, {.iparam = 0}) \ _(ERRINJ_INDEX_RESERVE, ERRINJ_BOOL, {.bparam = false})\ + _(ERRINJ_VY_MAX_TUPLE_SIZE, ERRINJ_INT, {.iparam = -1})\ ENUM0(errinj_id, ERRINJ_LIST); extern struct errinj errinjs[]; diff --git a/test/box/errinj.result b/test/box/errinj.result index 8090deedc..d107329f9 100644 --- a/test/box/errinj.result +++ b/test/box/errinj.result @@ -64,6 +64,7 @@ evals - ERRINJ_VY_LOG_FILE_RENAME: false - ERRINJ_VY_LOG_FLUSH: false - ERRINJ_VY_LOG_FLUSH_DELAY: false + - ERRINJ_VY_MAX_TUPLE_SIZE: -1 - ERRINJ_VY_POINT_ITER_WAIT: false - ERRINJ_VY_READ_PAGE: false - ERRINJ_VY_READ_PAGE_DELAY: false diff --git a/test/vinyl/gh-4864-stmt-alloc-fail-compact.result b/test/vinyl/gh-4864-stmt-alloc-fail-compact.result new file mode 100644 index 000000000..2c03697f6 --- /dev/null +++ b/test/vinyl/gh-4864-stmt-alloc-fail-compact.result @@ -0,0 +1,93 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... +fiber = require('fiber') + | --- + | ... +digest = require('digest') + | --- + | ... + +s = box.schema.space.create('test', {engine = 'vinyl'}) + | --- + | ... +_ = s:create_index('pk', {run_count_per_level = 100, page_size = 128, range_size = 1024}) + | --- + | ... + +test_run:cmd("setopt delimiter ';'") + | --- + | - true + | ... +function dump(big) + local step = big and 1 or 5 + for i = 1, 20, step do + s:replace{i, digest.urandom(1000)} + end + box.snapshot() +end; + | --- + | ... + +function compact() + s.index.pk:compact() + repeat + fiber.sleep(0.001) + local info = s.index.pk:stat() + until info.range_count == info.run_count +end; + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +-- The first run should be big enough to prevent major compaction +-- on the next dump, because run_count_per_level is ignored on the +-- last level. +-- +dump(true) + | --- + | ... +dump() + | --- + | ... +-- 1 range, 2 runs + +compact() + | --- + | ... +-- 1 range, 1 run after compaction + +dump() + | --- + | ... +-- 1 range, 2 runs + +errinj = box.error.injection + | --- + | ... +errinj.set('ERRINJ_VY_MAX_TUPLE_SIZE', 0) + | --- + | - ok + | ... +-- Should finish successfully despite vy_stmt_alloc() fail. +-- +compact() + | --- + | ... +-- 1 range, 1 run +s.index.pk:stat().range_count + | --- + | - 1 + | ... +s.index.pk:stat().run_count + | --- + | - 1 + | ... + +s:drop() + | --- + | ... diff --git a/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua b/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua new file mode 100644 index 000000000..53a050c9b --- /dev/null +++ b/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua @@ -0,0 +1,49 @@ +test_run = require('test_run').new() +fiber = require('fiber') +digest = require('digest') + +s = box.schema.space.create('test', {engine = 'vinyl'}) +_ = s:create_index('pk', {run_count_per_level = 100, page_size = 128, range_size = 1024}) + +test_run:cmd("setopt delimiter ';'") +function dump(big) + local step = big and 1 or 5 + for i = 1, 20, step do + s:replace{i, digest.urandom(1000)} + end + box.snapshot() +end; + +function compact() + s.index.pk:compact() + repeat + fiber.sleep(0.001) + local info = s.index.pk:stat() + until info.range_count == info.run_count +end; +test_run:cmd("setopt delimiter ''"); + +-- The first run should be big enough to prevent major compaction +-- on the next dump, because run_count_per_level is ignored on the +-- last level. +-- +dump(true) +dump() +-- 1 range, 2 runs + +compact() +-- 1 range, 1 run after compaction + +dump() +-- 1 range, 2 runs + +errinj = box.error.injection +errinj.set('ERRINJ_VY_MAX_TUPLE_SIZE', 0) +-- Should finish successfully despite vy_stmt_alloc() fail. +-- +compact() +-- 1 range, 1 run +s.index.pk:stat().range_count +s.index.pk:stat().run_count + +s:drop() diff --git a/test/vinyl/suite.ini b/test/vinyl/suite.ini index 1417d7156..ed602bb64 100644 --- a/test/vinyl/suite.ini +++ b/test/vinyl/suite.ini @@ -2,7 +2,7 @@ core = tarantool description = vinyl integration tests script = vinyl.lua -release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj_stat.test.lua errinj_tx.test.lua errinj_vylog.test.lua partial_dump.test.lua quota_timeout.test.lua recovery_quota.test.lua replica_rejoin.test.lua +release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj_stat.test.lua errinj_tx.test.lua errinj_vylog.test.lua partial_dump.test.lua quota_timeout.test.lua recovery_quota.test.lua replica_rejoin.test.lua gh-4864-stmt-alloc-fail-compact.test.lua config = suite.cfg lua_libs = suite.lua stress.lua large.lua txn_proxy.lua ../box/lua/utils.lua use_unix_sockets = True -- 2.17.1