From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id CC2E829F8B for ; Fri, 17 Aug 2018 04:32:15 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F4Wivdmf4NwJ for ; Fri, 17 Aug 2018 04:32:15 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 65E8429F6A for ; Fri, 17 Aug 2018 04:32:15 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/1] box: option to start tarantool with no format checks. References: <7f0572f5-bf79-2ac3-7828-82e9f34f93c9@tarantool.org> From: Vladislav Shpilevoy Message-ID: <5d40a08c-cdcd-22cc-b637-de357645a56c@tarantool.org> Date: Fri, 17 Aug 2018 11:32:12 +0300 MIME-Version: 1.0 In-Reply-To: <7f0572f5-bf79-2ac3-7828-82e9f34f93c9@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Imeev Mergen , tarantool-patches@freelists.org Cc: Kirill Yukhin 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@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 > 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 >      - >    - - 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 >      - >    - - log > @@ -115,6 +117,8 @@ cfg_filter(box.cfg) >      - false >    - - hot_standby >      - false > +  - - ignore_space_formats > +    - false >    - - listen >      - >    - - log >