[tarantool-patches] Re: [PATCH v2 1/1] box: option to start tarantool with no format checks.
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Aug 17 11:32:12 MSK 2018
LGTM.
On 17/08/2018 11:11, Imeev Mergen wrote:
>
>
> On 08/17/2018 12:08 AM, Vladislav Shpilevoy wrote:
>> Thanks for the patch! I have pushed my review fix
>> on a separate commit. Please, look at it, squash.
>> Then the patch will be lgtm.
> Thank you for review! I squashed and rebased. Also, I renamed
> created in test space to 't0' because some conflicts emerged
> after rebase.
>>
>> On 14/08/2018 14:50, imeevma at tarantool.org wrote:
>>> It is a common case that an instance is running on a version
>>> 1.6.*, then is upgraded to 1.9 with box.schema.upgrade(). But
>>> some of users has malformed space formats, and some of them got
>>> and ignored errors on box.schema.upgrade(). Such half-upgraded
>>> data can not be used to start a new version due to format
>>> violations, and can not be rolled back because some of new system
>>> spaces managed to be created. And we can not repair raw xlogs and
>>> snapshots because we have no such tools and they are zipped.
>>>
>>> So it would be useful to have a special box.cfg option
>>> ignore_space_formats. It would allow to start an instance on
>>> the spaces with malformed formats, fix them, create a snapshot
>>> and then start with the formats turned on.
>>>
>>> Closes #3605
>>>
>>> @TarantoolBot document
>>> Title: box.cfg option 'ignore_space_formats'
>>> The option allows to turn off space formats validation before
>>> the instance is started. Useful to fix malformed formats after
>>> an upgrade from version < 1.7.5 to >= 1.7.5.
>>> ---
>>> Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-3605-ignore-space-formats-option
>>> Issue: https://github.com/tarantool/tarantool/issues/3605
>>>
> commit 2124ae8057df48ec19161e9fa38bc70e8c09aa80
> Author: Mergen Imeev <imeevma at gmail.com>
> Date: Mon Aug 13 18:55:46 2018 +0300
>
> box: option to start tarantool with no format checks.
>
> It is a common case that an instance is running on a version
> 1.6.*, then is upgraded to 1.9 with box.schema.upgrade(). But
> some of users has malformed space formats, and some of them got
> and ignored errors on box.schema.upgrade(). Such half-upgraded
> data can not be used to start a new version due to format
> violations, and can not be rolled back because some of new system
> spaces managed to be created. And we can not repair raw xlogs and
> snapshots because we have no such tools and they are zipped.
>
> So it would be useful to have a special box.cfg option
> ignore_space_formats. It would allow to start an instance on
> the spaces with malformed formats, fix them, create a snapshot
> and then start with the formats turned on.
>
> Closes #3605
>
> @TarantoolBot document
> Title: box.cfg option 'ignore_space_formats'
> The option allows to turn off space formats validation before
> the instance is started. Useful to fix malformed formats after
> an upgrade from version < 1.7.5 to >= 1.7.5.
>
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index ec3b91b..b379dad 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -54,6 +54,18 @@
> #include "sequence.h"
>
> /**
> + * A flag to ignore space formats and do not validate tuples by
> + * them.
> + */
> +static bool ignore_space_formats = false;
> +
> +void
> +box_set_ignore_space_formats(bool value)
> +{
> + ignore_space_formats = value;
> +}
> +
> +/**
> * chap-sha1 of empty string, i.e.
> * base64_encode(sha1(sha1(""), 0)
> */
> @@ -533,7 +545,7 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t errcode,
> const char *space_opts;
> struct field_def *fields;
> uint32_t field_count;
> - if (dd_version_id >= version_id(1, 7, 6)) {
> + if (dd_version_id >= version_id(1, 7, 6) && !ignore_space_formats) {
> /* Check space opts. */
> space_opts =
> tuple_field_with_type_xc(tuple, BOX_SPACE_FIELD_OPTS,
> diff --git a/src/box/box.h b/src/box/box.h
> index 9dfb3fd..d4043db 100644
> --- a/src/box/box.h
> +++ b/src/box/box.h
> @@ -176,6 +176,7 @@ void box_set_vinyl_timeout(void);
> void box_set_replication_timeout(void);
> void box_set_replication_connect_timeout(void);
> void box_set_replication_connect_quorum(void);
> +void box_set_ignore_space_formats(bool value);
>
> extern "C" {
> #endif /* defined(__cplusplus) */
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 2a7142d..6682d49 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -66,6 +66,7 @@ local default_cfg = {
> username = nil,
> coredump = false,
> read_only = false,
> + ignore_space_formats= false,
> hot_standby = false,
> checkpoint_interval = 3600,
> checkpoint_count = 2,
> @@ -124,6 +125,7 @@ local template_cfg = {
> checkpoint_interval = 'number',
> checkpoint_count = 'number',
> read_only = 'boolean',
> + ignore_space_formats= 'boolean',
> hot_standby = 'boolean',
> worker_pool_threads = 'number',
> replication_timeout = 'number',
> diff --git a/src/main.cc b/src/main.cc
> index 1682bae..30cc78d 100644
> --- a/src/main.cc
> +++ b/src/main.cc
> @@ -521,6 +521,7 @@ load_cfg()
>
> title_set_custom(cfg_gets("custom_proc_title"));
> title_update();
> + box_set_ignore_space_formats(cfg_geti("ignore_space_formats") != 0);
> box_cfg();
> }
>
> diff --git a/test/app-tap/init_script.result b/test/app-tap/init_script.result
> index eea9f5b..f8b8994 100644
> --- a/test/app-tap/init_script.result
> +++ b/test/app-tap/init_script.result
> @@ -9,41 +9,42 @@ box.cfg
> 4 coredump:false
> 5 force_recovery:false
> 6 hot_standby:false
> -7 listen:port
> -8 log:tarantool.log
> -9 log_format:plain
> -10 log_level:5
> -11 log_nonblock:true
> -12 memtx_dir:.
> -13 memtx_max_tuple_size:1048576
> -14 memtx_memory:107374182
> -15 memtx_min_tuple_size:16
> -16 pid_file:box.pid
> -17 read_only:false
> -18 readahead:16320
> -19 replication_connect_timeout:30
> -20 replication_sync_lag:10
> -21 replication_timeout:1
> -22 rows_per_wal:500000
> -23 slab_alloc_factor:1.05
> -24 too_long_threshold:0.5
> -25 vinyl_bloom_fpr:0.05
> -26 vinyl_cache:134217728
> -27 vinyl_dir:.
> -28 vinyl_max_tuple_size:1048576
> -29 vinyl_memory:134217728
> -30 vinyl_page_size:8192
> -31 vinyl_range_size:1073741824
> -32 vinyl_read_threads:1
> -33 vinyl_run_count_per_level:2
> -34 vinyl_run_size_ratio:3.5
> -35 vinyl_timeout:60
> -36 vinyl_write_threads:2
> -37 wal_dir:.
> -38 wal_dir_rescan_delay:2
> -39 wal_max_size:268435456
> -40 wal_mode:write
> -41 worker_pool_threads:4
> +7 ignore_space_formats:false
> +8 listen:port
> +9 log:tarantool.log
> +10 log_format:plain
> +11 log_level:5
> +12 log_nonblock:true
> +13 memtx_dir:.
> +14 memtx_max_tuple_size:1048576
> +15 memtx_memory:107374182
> +16 memtx_min_tuple_size:16
> +17 pid_file:box.pid
> +18 read_only:false
> +19 readahead:16320
> +20 replication_connect_timeout:30
> +21 replication_sync_lag:10
> +22 replication_timeout:1
> +23 rows_per_wal:500000
> +24 slab_alloc_factor:1.05
> +25 too_long_threshold:0.5
> +26 vinyl_bloom_fpr:0.05
> +27 vinyl_cache:134217728
> +28 vinyl_dir:.
> +29 vinyl_max_tuple_size:1048576
> +30 vinyl_memory:134217728
> +31 vinyl_page_size:8192
> +32 vinyl_range_size:1073741824
> +33 vinyl_read_threads:1
> +34 vinyl_run_count_per_level:2
> +35 vinyl_run_size_ratio:3.5
> +36 vinyl_timeout:60
> +37 vinyl_write_threads:2
> +38 wal_dir:.
> +39 wal_dir_rescan_delay:2
> +40 wal_max_size:268435456
> +41 wal_mode:write
> +42 worker_pool_threads:4
> --
> -- Test insert from detached fiber
> --
> diff --git a/test/box-tap/ignore_formats.test.lua b/test/box-tap/ignore_formats.test.lua
> new file mode 100755
> index 0000000..14211d3
> --- /dev/null
> +++ b/test/box-tap/ignore_formats.test.lua
> @@ -0,0 +1,24 @@
> +#!/usr/bin/env tarantool
> +--
> +-- gh-3605: allow to ignore space formats via box.cfg option.
> +--
> +local tap = require('tap')
> +local test = tap.test("ignore_formats")
> +test:plan(3)
> +
> +box.cfg{ignore_space_formats=true}
> +
> +local format = {}
> +format[1] = {'field1', 'unsigned'}
> +format[2] = {'field2', 'unsigned'}
> +
> +local s = box.schema.create_space('t0', {format = format})
> +local pk = s:create_index('pk')
> +test:ok(pcall(s.replace, s, {1, 'string'}), 'can violate format')
> +-- Is not a dynamic option.
> +local ok, err = pcall(box.cfg, {ignore_space_formats = false})
> +test:ok(not ok, "ignore_space_formats is not a dynamic option")
> +test:ok(err:match("Can't set option"), "error message")
> +
> +s:drop()
> +os.exit(test:check() == true and 0 or 1)
> diff --git a/test/box/admin.result b/test/box/admin.result
> index c3e318a..9200ed8 100644
> --- a/test/box/admin.result
> +++ b/test/box/admin.result
> @@ -30,6 +30,8 @@ cfg_filter(box.cfg)
> - false
> - - hot_standby
> - false
> + - - ignore_space_formats
> + - false
> - - listen
> - <hidden>
> - - log
> diff --git a/test/box/cfg.result b/test/box/cfg.result
> index cda7aa0..fad4b87 100644
> --- a/test/box/cfg.result
> +++ b/test/box/cfg.result
> @@ -26,6 +26,8 @@ cfg_filter(box.cfg)
> - false
> - - hot_standby
> - false
> + - - ignore_space_formats
> + - false
> - - listen
> - <hidden>
> - - log
> @@ -115,6 +117,8 @@ cfg_filter(box.cfg)
> - false
> - - hot_standby
> - false
> + - - ignore_space_formats
> + - false
> - - listen
> - <hidden>
> - - log
>
More information about the Tarantool-patches
mailing list