[tarantool-patches] [PATCH] sql: inherit global parameters for vinyl spaces

[tarantool-patches] [PATCH] sql: inherit global parameters for vinyl spaces

[Nikita Pettik]

It is possible to create vinyl tables, if pragma sql_default_engine is
set to corresponding engine. However, such spaces don't inherit global
vinyl-specific options which can be set by box.cfg{}. Lets fix it and
during encoding of index options fetch appropriate configurations from
cfg.

Closes #3912
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3912-sql-vinyl-inherit-params
Issue: https://github.com/tarantool/tarantool/issues/3912

 src/box/sql.c                | 47 +++++++++++++++++++++++++++++--------
 test/sql/engine.cfg          |  3 +++
 test/sql/vinyl-opts-cfg.lua  | 14 +++++++++++
 test/sql/vinyl-opts.result   | 55 ++++++++++++++++++++++++++++++++++++++++++++
 test/sql/vinyl-opts.test.lua | 17 ++++++++++++++
 5 files changed, 126 insertions(+), 10 deletions(-)
 create mode 100644 test/sql/vinyl-opts-cfg.lua
 create mode 100644 test/sql/vinyl-opts.result
 create mode 100644 test/sql/vinyl-opts.test.lua

diff --git a/src/box/sql.c b/src/box/sql.c
index b339d1371..91286f822 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -30,6 +30,7 @@
  */
 #include <assert.h>
 #include "field_def.h"
+#include "cfg.h"
 #include "sql.h"
 #include "sql/sqlInt.h"
 #include "sql/tarantoolInt.h"
@@ -1167,18 +1168,44 @@ char *
 sql_encode_index_opts(struct region *region, const struct index_opts *opts,
 		      uint32_t *size)
 {
-	int unique_len = strlen("unique");
-	*size = mp_sizeof_map(1) + mp_sizeof_str(unique_len) +
-		mp_sizeof_bool(opts->is_unique);
-	char *mem = (char *) region_alloc(region, *size);
-	if (mem == NULL) {
-		diag_set(OutOfMemory, *size, "region_alloc", "mem");
+	size_t used = region_used(region);
+	struct mpstream stream;
+	bool is_error = false;
+	mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+		      set_encode_error, &is_error);
+	/*
+	 * In case of vinyl engine we must inherit global
+	 * (i.e. set via box.cfg) params such as bloom_fpr,
+	 * page_size etc.
+	 */
+	uint8_t current_engine = current_session()->sql_default_engine;
+	uint32_t map_sz = current_engine == SQL_STORAGE_ENGINE_VINYL ? 6 : 1;
+	mpstream_encode_map(&stream, map_sz);
+	mpstream_encode_str(&stream, "unique");
+	mpstream_encode_bool(&stream, opts->is_unique);
+	if (current_engine == SQL_STORAGE_ENGINE_VINYL) {
+		mpstream_encode_str(&stream, "range_size");
+		mpstream_encode_uint(&stream, cfg_geti64("vinyl_range_size"));
+		mpstream_encode_str(&stream, "page_size");
+		mpstream_encode_uint(&stream, cfg_geti64("vinyl_page_size"));
+		mpstream_encode_str(&stream, "run_count_per_level");
+		mpstream_encode_uint(&stream, cfg_geti("vinyl_run_count_per_level"));
+		mpstream_encode_str(&stream, "run_size_ratio");
+		mpstream_encode_double(&stream, cfg_getd("vinyl_run_size_ratio"));
+		mpstream_encode_str(&stream, "bloom_fpr");
+		mpstream_encode_double(&stream, cfg_getd("vinyl_bloom_fpr"));
+	}
+	mpstream_flush(&stream);
+	if (is_error) {
+		diag_set(OutOfMemory, stream.pos - stream.buf,
+			 "mpstream_flush", "stream");
 		return NULL;
 	}
-	char *pos = mp_encode_map(mem, 1);
-	pos = mp_encode_str(pos, "unique", unique_len);
-	pos = mp_encode_bool(pos, opts->is_unique);
-	return mem;
+	*size = region_used(region) - used;
+	char *raw = region_join(region, *size);
+	if (raw == NULL)
+		diag_set(OutOfMemory, *size, "region_join", "raw");
+	return raw;
 }
 
 void
diff --git a/test/sql/engine.cfg b/test/sql/engine.cfg
index 0007d8d7a..d5666a081 100644
--- a/test/sql/engine.cfg
+++ b/test/sql/engine.cfg
@@ -1,4 +1,7 @@
 {
+    "vinyl-opts.test.lua" : {
+        "vinyl": {"engine": "vinyl"}
+    },
     "*": {
         "memtx": {"engine": "memtx"},
         "vinyl": {"engine": "vinyl"}
diff --git a/test/sql/vinyl-opts-cfg.lua b/test/sql/vinyl-opts-cfg.lua
new file mode 100644
index 000000000..0a8256ca3
--- /dev/null
+++ b/test/sql/vinyl-opts-cfg.lua
@@ -0,0 +1,14 @@
+#!/usr/bin/env tarantool
+
+-- Set of custom vinyl params, which are used in the test
+-- of the same name (vinyl-opts.test.lua).
+--
+box.cfg {
+    vinyl_bloom_fpr = 0.1,
+    vinyl_page_size = 32 * 1024,
+    vinyl_range_size = 512 * 1024 * 1024,
+    vinyl_run_size_ratio = 5,
+    vinyl_run_count_per_level = 3
+}
+
+require('console').listen(os.getenv('ADMIN'))
diff --git a/test/sql/vinyl-opts.result b/test/sql/vinyl-opts.result
new file mode 100644
index 000000000..4e6c4bc85
--- /dev/null
+++ b/test/sql/vinyl-opts.result
@@ -0,0 +1,55 @@
+test_run = require('test_run').new()
+---
+...
+test_run:cmd("create server test with script='sql/vinyl-opts-cfg.lua'")
+---
+- true
+...
+test_run:cmd("start server test")
+---
+- true
+...
+test_run:cmd("switch test")
+---
+- true
+...
+box.sql.execute('pragma sql_default_engine= \'vinyl\'')
+---
+...
+box.sql.execute('CREATE TABLE v1 (id INT PRIMARY KEY, b INT);')
+---
+...
+box.space.V1.index[0].options
+---
+- page_size: 32768
+  run_count_per_level: 3
+  run_size_ratio: 5
+  bloom_fpr: 0.1
+  range_size: 536870912
+...
+box.sql.execute('CREATE INDEX i1 ON v1(b);')
+---
+...
+box.space.V1.index[1].options
+---
+- page_size: 32768
+  run_count_per_level: 3
+  run_size_ratio: 5
+  bloom_fpr: 0.1
+  range_size: 536870912
+...
+box.space.V1:drop()
+---
+...
+test_run:cmd('switch default')
+---
+- true
+...
+test_run:cmd("stop server test")
+---
+- true
+...
+test_run:cmd("cleanup server test")
+---
+- true
+...
diff --git a/test/sql/vinyl-opts.test.lua b/test/sql/vinyl-opts.test.lua
new file mode 100644
index 000000000..843693bca
--- /dev/null
+++ b/test/sql/vinyl-opts.test.lua
@@ -0,0 +1,17 @@
+test_run = require('test_run').new()
+test_run:cmd("create server test with script='sql/vinyl-opts-cfg.lua'")
+test_run:cmd("start server test")
+test_run:cmd("switch test")
+
+box.sql.execute('pragma sql_default_engine= \'vinyl\'')
+box.sql.execute('CREATE TABLE v1 (id INT PRIMARY KEY, b INT);')
+box.space.V1.index[0].options
+
+box.sql.execute('CREATE INDEX i1 ON v1(b);')
+box.space.V1.index[1].options
+
+box.space.V1:drop()
+
+test_run:cmd('switch default')
+test_run:cmd("stop server test")
+test_run:cmd("cleanup server test")
-- 
2.15.1

[tarantool-patches] Re: [PATCH] sql: inherit global parameters for vinyl spaces

[Vladislav Shpilevoy]

LGTM.

On 20/02/2019 17:53, Nikita Pettik wrote:
> It is possible to create vinyl tables, if pragma sql_default_engine is
> set to corresponding engine. However, such spaces don't inherit global
> vinyl-specific options which can be set by box.cfg{}. Lets fix it and
> during encoding of index options fetch appropriate configurations from
> cfg.
> 
> Closes #3912
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3912-sql-vinyl-inherit-params
> Issue: https://github.com/tarantool/tarantool/issues/3912
> 

[tarantool-patches] Re: [PATCH] sql: inherit global parameters for vinyl spaces

[Kirill Yukhin]

Hello,

On 20 Feb 17:53, Nikita Pettik wrote:
> It is possible to create vinyl tables, if pragma sql_default_engine is
> set to corresponding engine. However, such spaces don't inherit global
> vinyl-specific options which can be set by box.cfg{}. Lets fix it and
> during encoding of index options fetch appropriate configurations from
> cfg.
> 
> Closes #3912
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3912-sql-vinyl-inherit-params
> Issue: https://github.com/tarantool/tarantool/issues/3912

I've checked yout patch into 2.1 branch.

--
Regards, Kirill Yukhin