From 632592bf21da6a1f2f666dc186f4ec4f4319e43f Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Date: Sun, 18 Feb 2018 17:05:01 +0300
Subject: [PATCH] Check vinyl index options on box cfg and space alter
Currently, one can set insane values for most vinyl index options, which
will most certainly result in a crash (e.g. bloom_fpr = 100). Add some
sanity checks.
---
src/box/alter.cc | 30 +++++++++++---
src/box/box.cc | 52 ++++++++++++++++++++-----
test/box-tap/cfg.test.lua | 40 +++++--------------
test/vinyl/ddl.result | 40 +++++++++++++++++++
test/vinyl/ddl.test.lua | 11 ++++++
test/vinyl/write_iterator_rand.result | 16 ++++----
test/vinyl/write_iterator_rand.test.lua | 16 ++++----
7 files changed, 144 insertions(+), 61 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index e66863cbf1..af9fbb52b8 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -236,13 +236,33 @@ index_opts_decode(struct index_opts *opts, const char *map)
BOX_INDEX_FIELD_OPTS, "distance must be either "\
"'euclid' or 'manhattan'");
}
- if (opts->run_count_per_level <= 0)
+ if (opts->range_size <= 0) {
tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
BOX_INDEX_FIELD_OPTS,
- "run_count_per_level must be > 0");
- if (opts->run_size_ratio <= 1)
- tnt_raise(ClientError, ER_WRONG_SPACE_OPTIONS,
- BOX_INDEX_FIELD_OPTS, "run_size_ratio must be > 1");
+ "range_size must be greater than 0");
+ }
+ if (opts->page_size <= 0 || opts->page_size > opts->range_size) {
+ tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
+ BOX_INDEX_FIELD_OPTS,
+ "page_size must be greater than 0 and "
+ "less than or equal to range_size");
+ }
+ if (opts->run_count_per_level <= 0) {
+ tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
+ BOX_INDEX_FIELD_OPTS,
+ "run_count_per_level must be greater than 0");
+ }
+ if (opts->run_size_ratio <= 1) {
+ tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
+ BOX_INDEX_FIELD_OPTS,
+ "run_size_ratio must be greater than 1");
+ }
+ if (opts->bloom_fpr <= 0 || opts->bloom_fpr > 1) {
+ tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
+ BOX_INDEX_FIELD_OPTS,
+ "bloom_fpr must be greater than 0 and "
+ "less than or equal to 1");
+ }
}
/**
diff --git a/src/box/box.cc b/src/box/box.cc
index 33d237bcb3..32b08e3600 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -490,6 +490,48 @@ box_check_wal_max_size(int64_t wal_max_size)
return wal_max_size;
}
+static void
+box_check_vinyl_options(void)
+{
+ int read_threads = cfg_geti("vinyl_read_threads");
+ int write_threads = cfg_geti("vinyl_write_threads");
+ int64_t range_size = cfg_geti64("vinyl_range_size");
+ int64_t page_size = cfg_geti64("vinyl_page_size");
+ int run_count_per_level = cfg_geti("vinyl_run_count_per_level");
+ double run_size_ratio = cfg_getd("vinyl_run_size_ratio");
+ double bloom_fpr = cfg_getd("vinyl_bloom_fpr");
+
+ if (read_threads < 1) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_read_threads",
+ "must be greater than or equal to 1");
+ }
+ if (write_threads < 2) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_write_threads",
+ "must be greater than or equal to 2");
+ }
+ if (range_size <= 0) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_range_size",
+ "must be greater than 0");
+ }
+ if (page_size <= 0 || page_size > range_size) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_page_size",
+ "must be greater than 0 and less than "
+ "or equal to vinyl_range_size");
+ }
+ if (run_count_per_level <= 0) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_run_count_per_level",
+ "must be greater than 0");
+ }
+ if (run_size_ratio <= 1) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_run_size_ratio",
+ "must be greater than 1");
+ }
+ if (bloom_fpr <= 0 || bloom_fpr > 1) {
+ tnt_raise(ClientError, ER_CFG, "vinyl_bloom_fpr",
+ "must be greater than 0 and less than or equal to 1");
+ }
+}
+
void
box_check_config()
{
@@ -510,15 +552,7 @@ box_check_config()
box_check_wal_max_size(cfg_geti64("wal_max_size"));
box_check_wal_mode(cfg_gets("wal_mode"));
box_check_memtx_min_tuple_size(cfg_geti64("memtx_min_tuple_size"));
- if (cfg_geti64("vinyl_page_size") > cfg_geti64("vinyl_range_size"))
- tnt_raise(ClientError, ER_CFG, "vinyl_page_size",
- "can't be greater than vinyl_range_size");
- if (cfg_geti("vinyl_read_threads") < 1)
- tnt_raise(ClientError, ER_CFG,
- "vinyl_read_threads", "must be >= 1");
- if (cfg_geti("vinyl_write_threads") < 2)
- tnt_raise(ClientError, ER_CFG,
- "vinyl_write_threads", "must be >= 2");
+ box_check_vinyl_options();
}
/*
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index 90dc04bd30..453b616dd5 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
local fio = require('fio')
local uuid = require('uuid')
local msgpack = require('msgpack')
-test:plan(85)
+test:plan(89)
--------------------------------------------------------------------------------
-- Invalid values
@@ -38,6 +38,14 @@ invalid('listen', '//!')
invalid('log', ':')
invalid('log', 'syslog:xxx=')
invalid('log_level', 'unknown')
+invalid('vinyl_read_threads', 0)
+invalid('vinyl_write_threads', 1)
+invalid('vinyl_range_size', 0)
+invalid('vinyl_page_size', 0)
+invalid('vinyl_run_count_per_level', 0)
+invalid('vinyl_run_size_ratio', 1)
+invalid('vinyl_bloom_fpr', 0)
+invalid('vinyl_bloom_fpr', 1.1)
test:is(type(box.cfg), 'function', 'box is not started')
@@ -264,36 +272,6 @@ os.exit(0)
]]
test:is(run_script(code), 0, "page size equal with range")
---
--- there is at least one vinyl reader thread.
---
-code = [[
-box.cfg{vinyl_read_threads = 0}
-os.exit(0)
-]]
-test:is(run_script(code), PANIC, "vinyl_read_threads = 0")
-
-code = [[
-box.cfg{vinyl_read_threads = 1}
-os.exit(0)
-]]
-test:is(run_script(code), 0, "vinyl_read_threads = 1")
-
---
--- gh-2150 one vinyl worker thread is reserved for dumps
---
-code = [[
-box.cfg{vinyl_write_threads = 1}
-os.exit(0)
-]]
-test:is(run_script(code), PANIC, "vinyl_write_threads = 1")
-
-code = [[
-box.cfg{vinyl_write_threads = 2}
-os.exit(0)
-]]
-test:is(run_script(code), 0, "vinyl_write_threads = 2")
-
-- test memtx options upgrade
code = [[
box.cfg{slab_alloc_arena = 0.2, slab_alloc_minimal = 16,
diff --git a/test/vinyl/ddl.result b/test/vinyl/ddl.result
index 17c5c5ca37..64e1b77cb6 100644
--- a/test/vinyl/ddl.result
+++ b/test/vinyl/ddl.result
@@ -4,6 +4,46 @@ fiber = require('fiber')
test_run = require('test_run').new()
---
...
+-- sanity checks
+space = box.schema.space.create('test', {engine = 'vinyl' })
+---
+...
+space:create_index('pk', {range_size = 0})
+---
+- error: 'Wrong index options (field 4): range_size must be greater than 0'
+...
+space:create_index('pk', {page_size = 0})
+---
+- error: 'Wrong index options (field 4): page_size must be greater than 0 and less
+ than or equal to range_size'
+...
+space:create_index('pk', {page_size = 8192, range_size = 4096})
+---
+- error: 'Wrong index options (field 4): page_size must be greater than 0 and less
+ than or equal to range_size'
+...
+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})
+---
+- error: 'Wrong index options (field 4): run_size_ratio must be greater than 1'
+...
+space:create_index('pk', {bloom_fpr = 0})
+---
+- error: 'Wrong index options (field 4): bloom_fpr must be greater than 0 and less
+ than or equal to 1'
+...
+space:create_index('pk', {bloom_fpr = 1.1})
+---
+- error: 'Wrong index options (field 4): bloom_fpr must be greater than 0 and less
+ than or equal to 1'
+...
+space:drop()
+---
+...
-- space secondary index create
space = box.schema.space.create('test', { engine = 'vinyl' })
---
diff --git a/test/vinyl/ddl.test.lua b/test/vinyl/ddl.test.lua
index 98a19653f3..e0f453bd91 100644
--- a/test/vinyl/ddl.test.lua
+++ b/test/vinyl/ddl.test.lua
@@ -1,6 +1,17 @@
fiber = require('fiber')
test_run = require('test_run').new()
+-- sanity checks
+space = box.schema.space.create('test', {engine = 'vinyl' })
+space:create_index('pk', {range_size = 0})
+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', {bloom_fpr = 0})
+space:create_index('pk', {bloom_fpr = 1.1})
+space:drop()
+
-- space secondary index create
space = box.schema.space.create('test', { engine = 'vinyl' })
index1 = space:create_index('primary')
diff --git a/test/vinyl/write_iterator_rand.result b/test/vinyl/write_iterator_rand.result
index c45df7bdd2..e1affa7a53 100644
--- a/test/vinyl/write_iterator_rand.result
+++ b/test/vinyl/write_iterator_rand.result
@@ -198,40 +198,40 @@ test_run:cmd("setopt delimiter ''");
- true
...
-- Tests on write iterator with random combinations of page_size and range_size
-range_size = math.random(128, 256)
+page_size = math.random(128, 256)
---
...
-page_size = range_size * math.random(10, 20)
+range_size = page_size * math.random(10, 20)
---
...
fill_space_with_sizes(page_size, range_size, 300)
---
- ok
...
-range_size = math.random(256, 512)
+page_size = math.random(256, 512)
---
...
-page_size = range_size * math.random(10, 20)
+range_size = page_size * math.random(10, 20)
---
...
fill_space_with_sizes(page_size, range_size, 500)
---
- ok
...
-range_size = math.random(512, 1024)
+page_size = math.random(512, 1024)
---
...
-page_size = range_size * math.random(10, 20)
+range_size = page_size * math.random(10, 20)
---
...
fill_space_with_sizes(page_size, range_size, 700)
---
- ok
...
-range_size = math.random(1024, 2048)
+page_size = math.random(1024, 2048)
---
...
-page_size = range_size * math.random(10, 20)
+range_size = page_size * math.random(10, 20)
---
...
fill_space_with_sizes(page_size, range_size, 900)
diff --git a/test/vinyl/write_iterator_rand.test.lua b/test/vinyl/write_iterator_rand.test.lua
index ae2abec344..345b1d99d0 100644
--- a/test/vinyl/write_iterator_rand.test.lua
+++ b/test/vinyl/write_iterator_rand.test.lua
@@ -187,18 +187,18 @@ test_run:cmd("setopt delimiter ''");
-- Tests on write iterator with random combinations of page_size and range_size
-range_size = math.random(128, 256)
-page_size = range_size * math.random(10, 20)
+page_size = math.random(128, 256)
+range_size = page_size * math.random(10, 20)
fill_space_with_sizes(page_size, range_size, 300)
-range_size = math.random(256, 512)
-page_size = range_size * math.random(10, 20)
+page_size = math.random(256, 512)
+range_size = page_size * math.random(10, 20)
fill_space_with_sizes(page_size, range_size, 500)
-range_size = math.random(512, 1024)
-page_size = range_size * math.random(10, 20)
+page_size = math.random(512, 1024)
+range_size = page_size * math.random(10, 20)
fill_space_with_sizes(page_size, range_size, 700)
-range_size = math.random(1024, 2048)
-page_size = range_size * math.random(10, 20)
+page_size = math.random(1024, 2048)
+range_size = page_size * math.random(10, 20)
fill_space_with_sizes(page_size, range_size, 900)
--
GitLab