Hi! Yes, here is the new patch. Change the minimum possible value of the variable vinyl_run_size_ratio to 2. Closes #3346 issue: https://github.com/tarantool/tarantool/issues/3346 ---  src/box/alter.cc | 4 ++--  src/box/box.cc | 4 ++--  src/box/vy_range.c | 2 +-  test/box-tap/cfg.test.lua | 2 +-  test/vinyl/ddl.result | 5 +++--  test/vinyl/ddl.test.lua | 2 +-  6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index bef25b605..123e94994 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -227,10 +227,10 @@ index_opts_decode(struct index_opts *opts, const char *map,   "run_count_per_level must be greater than 0");   return -1;   } - if (opts->run_size_ratio <= 1) { + if (opts->run_size_ratio < 2) {   diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,   BOX_INDEX_FIELD_OPTS, - "run_size_ratio must be greater than 1"); + "run_size_ratio must be greater than or uqual to 2");   return -1;   }   if (opts->bloom_fpr <= 0 || opts->bloom_fpr > 1) { diff --git a/src/box/box.cc b/src/box/box.cc index b119c927b..0f26e28cd 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -589,9 +589,9 @@ box_check_vinyl_options(void)   tnt_raise(ClientError, ER_CFG, "vinyl_run_count_per_level",   "must be greater than 0");   } - if (run_size_ratio <= 1) { + if (run_size_ratio < 2) {   tnt_raise(ClientError, ER_CFG, "vinyl_run_size_ratio", - "must be greater than 1"); + "must be greater than or uqual to 2");   }   if (bloom_fpr <= 0 || bloom_fpr > 1) {   tnt_raise(ClientError, ER_CFG, "vinyl_bloom_fpr", diff --git a/src/box/vy_range.c b/src/box/vy_range.c index 4ff852112..8d6bfd30b 100644 --- a/src/box/vy_range.c +++ b/src/box/vy_range.c @@ -289,7 +289,7 @@ vy_range_update_compaction_priority(struct vy_range *range,   const struct index_opts *opts)  {   assert(opts->run_count_per_level > 0); - assert(opts->run_size_ratio > 1); + assert(opts->run_size_ratio >= 2);     range->compaction_priority = 0;   vy_disk_stmt_counter_reset(&range->compaction_queue); diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua index d529447bb..b68c6f17b 100755 --- a/test/box-tap/cfg.test.lua +++ b/test/box-tap/cfg.test.lua @@ -46,7 +46,7 @@ invalid('vinyl_read_threads', 0)  invalid('vinyl_write_threads', 1)  invalid('vinyl_page_size', 0)  invalid('vinyl_run_count_per_level', 0) -invalid('vinyl_run_size_ratio', 1) +invalid('vinyl_run_size_ratio', 1.9)  invalid('vinyl_bloom_fpr', 0)  invalid('vinyl_bloom_fpr', 1.1)   diff --git a/test/vinyl/ddl.result b/test/vinyl/ddl.result index 9c453a007..a97e415be 100644 --- a/test/vinyl/ddl.result +++ b/test/vinyl/ddl.result @@ -23,9 +23,10 @@ space:create_index('pk', {run_count_per_level = 0})  - error: 'Wrong index options (field 4): run_count_per_level must be greater than      0'  ... -space:create_index('pk', {run_size_ratio = 1}) +space:create_index('pk', {run_size_ratio = 1.9})  --- -- error: 'Wrong index options (field 4): run_size_ratio must be greater than 1' +- error: 'Wrong index options (field 4): run_size_ratio must be greater than or uqual + to 2'  ...  space:create_index('pk', {bloom_fpr = 0})  --- diff --git a/test/vinyl/ddl.test.lua b/test/vinyl/ddl.test.lua index 010ec6d79..35e37a84f 100644 --- a/test/vinyl/ddl.test.lua +++ b/test/vinyl/ddl.test.lua @@ -6,7 +6,7 @@ space = box.schema.space.create('test', {engine = 'vinyl' })  space:create_index('pk', {page_size = 0})  space:create_index('pk', {page_size = 8192, range_size = 4096})  space:create_index('pk', {run_count_per_level = 0}) -space:create_index('pk', {run_size_ratio = 1}) +space:create_index('pk', {run_size_ratio = 1.9})  space:create_index('pk', {bloom_fpr = 0})  space:create_index('pk', {bloom_fpr = 1.1})  space:drop() -- 2.17.1 >Среда, 15 января 2020, 20:36 +03:00 от Nikita Pettik < korablev@tarantool.org >: > >On 13 Jan 12:48, Maxim Kulis wrote: >> Hi! >> Thanks for your review. >> > >Hi, > >Are you going to update your patch according to the review notes? >Review process is described here: >https://www.tarantool.io/en/doc/2.2/dev_guide/developer_guidelines/#how-to-submit-a-patch-for-review >  >> >Четверг, 5 декабря 2019, 14:27 +03:00 от Nikita Pettik < korablev@tarantool.org >: >> > >> >On 02 Dec 19:11, Maksim Kulis wrote: >> >> Change the minimum possible value of the variable vinyl_run_size_ratio to 2. >> >> >> >> Closes #3346 >> >> --- >> >> src/box/box.cc | 4 ++-- >> >> test/box-tap/cfg.test.lua | 2 +- >> >> 2 files changed, 3 insertions(+), 3 deletions(-) >> > >> >Hello. Thanks for the patch. >> > >> >Firsly, please update check of run_size_ratio during index options >> >decode (see index_opts_decode()). >> > >> >I would also update assert() in vy_range_update_compaction_priority(): >> >let's make it check that run_size_ratio always >= 2. >> > >> >Finally, I would add brief explanation to the commit message why >> >run_size_ratio less than 2 is meaningless (for those who are not >> >familiar with vinyl inner organization). >> >  >> >> diff --git a/src/box/box.cc b/src/box/box.cc >> >> index be2390335..e426fdab8 100644 >> >> --- a/src/box/box.cc >> >> +++ b/src/box/box.cc >> >> @@ -589,9 +589,9 @@ box_check_vinyl_options(void) >> >> tnt_raise(ClientError, ER_CFG, "vinyl_run_count_per_level", >> >> "must be greater than 0"); >> >> } >> >> - if (run_size_ratio <= 1) { >> >> + if (run_size_ratio < 2) { >> >> tnt_raise(ClientError, ER_CFG, "vinyl_run_size_ratio", >> >> - "must be greater than 1"); >> >> + "must be greater than or uqual to 2"); >> >> } >> >> if (bloom_fpr <= 0 || bloom_fpr > 1) { >> >> tnt_raise(ClientError, ER_CFG, "vinyl_bloom_fpr", >> >> diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua >> >> index d529447bb..b68c6f17b 100755 >> >> --- a/test/box-tap/cfg.test.lua >> >> +++ b/test/box-tap/cfg.test.lua >> >> @@ -46,7 +46,7 @@ invalid('vinyl_read_threads', 0) >> >> invalid('vinyl_write_threads', 1) >> >> invalid('vinyl_page_size', 0) >> >> invalid('vinyl_run_count_per_level', 0) >> >> -invalid('vinyl_run_size_ratio', 1) >> >> +invalid('vinyl_run_size_ratio', 1.9) >> >> invalid('vinyl_bloom_fpr', 0) >> >> invalid('vinyl_bloom_fpr', 1.1) >> >> >> >> -- >> >> 2.17.1 >> >> >> >> >> -- >> Maxim Kulis -- Maxim Kulis