Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: tarantool-patches@freelists.org
Cc: vdavydov.dev@gmail.com
Subject: Re: [tarantool-patches] [PATCH 1/1] netbox: don't cancel pending requests on schema change
Date: Fri, 6 Apr 2018 17:01:35 +0300	[thread overview]
Message-ID: <20180406140135.GC18946@atlas> (raw)
In-Reply-To: <b9a5316db2b28738d12c9847f01f95c1a9b4ad81.1523022354.git.v.shpilevoy@tarantool.org>

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/04/06 16:51]:
> 
> A request must be retried only if a server responded with
> WRONG_SCHEMA_VERSION error exactly to this request.

OK to push.

Please commit to 1.10.

I wonder how this didn't fire earlier.

> 
> Closes #3325
> ---
> Issue: https://github.com/tarantool/tarantool/issues/3325
> Branch: https://github.com/tarantool/tarantool/tree/gh-3325-do-not-cancel-netbox-by-schema-version
> 
>  src/box/lua/net_box.lua  | 26 ++++++----------------
>  test/box/errinj.result   | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  test/box/errinj.test.lua | 24 ++++++++++++++++++++
>  3 files changed, 88 insertions(+), 19 deletions(-)
> 
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index cf7b672c7..6edec37e1 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -150,29 +150,18 @@ local function create_transport(host, port, user, password, callback)
>      local recv_buf         = buffer.ibuf(buffer.READAHEAD)
>  
>      -- STATE SWITCHING --
> -    local function set_state(new_state, new_errno, new_error, schema_version)
> +    local function set_state(new_state, new_errno, new_error)
>          state = new_state
>          last_errno = new_errno
>          last_error = new_error
>          callback('state_changed', new_state, new_errno, new_error)
>          state_cond:broadcast()
> -        if state ~= 'active' then
> -            -- cancel all requests but the ones bearing the particular
> -            -- schema id; if schema id was omitted or we aren't fetching
> -            -- schema, cancel everything
> -            if not schema_version or state ~= 'fetch_schema' then
> -                schema_version = -1
> -            end
> -            local next_id, next_request = next(requests)
> -            while next_id do
> -                local id, request = next_id, next_request
> -                next_id, next_request = next(requests, id)
> -                if request.schema_version ~= schema_version then
> -                    requests[id] = nil -- this marks the request as completed
> -                    request.errno  = new_errno
> -                    request.response = new_error
> -                end
> +        if state == 'error' or state == 'error_reconnect' then
> +            for _, request in pairs(requests) do
> +                request.errno = new_errno
> +                request.response = new_error
>              end
> +            requests = {}
>          end
>      end
>  
> @@ -503,8 +492,7 @@ local function create_transport(host, port, user, password, callback)
>              local body
>              body, body_end = decode(body_rpos)
>              set_state('fetch_schema',
> -                      E_WRONG_SCHEMA_VERSION, body[IPROTO_ERROR_KEY],
> -                      response_schema_version)
> +                      E_WRONG_SCHEMA_VERSION, body[IPROTO_ERROR_KEY])
>              return iproto_schema_sm(schema_version)
>          end
>          return iproto_sm(schema_version)
> diff --git a/test/box/errinj.result b/test/box/errinj.result
> index 1cb5c2329..55e93c4b2 100644
> --- a/test/box/errinj.result
> +++ b/test/box/errinj.result
> @@ -1100,6 +1100,63 @@ errinj.set("ERRINJ_IPROTO_TX_DELAY", false)
>  s:drop()
>  ---
>  ...
> +--
> +-- gh-3325: do not cancel already sent requests, when a schema
> +-- change is detected.
> +--
> +s = box.schema.create_space('test')
> +---
> +...
> +pk = s:create_index('pk')
> +---
> +...
> +s:replace{1, 1}
> +---
> +- [1, 1]
> +...
> +cn = net_box.connect(box.cfg.listen)
> +---
> +...
> +errinj.set("ERRINJ_WAL_DELAY", true)
> +---
> +- ok
> +...
> +ok = nil
> +---
> +...
> +err = nil
> +---
> +...
> +test_run:cmd('setopt delimiter ";"')
> +---
> +- true
> +...
> +f = fiber.create(function()
> +  local str = 'box.space.test:create_index("sk", {parts = {{2, "integer"}}})'
> +  ok, err = pcall(cn.eval, cn, str)
> +end)
> +test_run:cmd('setopt delimiter ""');
> +---
> +...
> +cn.space.test:get{1}
> +---
> +- [1, 1]
> +...
> +errinj.set("ERRINJ_WAL_DELAY", false)
> +---
> +- ok
> +...
> +ok, err
> +---
> +- true
> +- null
> +...
> +cn:close()
> +---
> +...
> +s:drop()
> +---
> +...
>  box.schema.user.revoke('guest', 'read,write,execute','universe')
>  ---
>  ...
> diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
> index 3af1b74dc..09df00dcd 100644
> --- a/test/box/errinj.test.lua
> +++ b/test/box/errinj.test.lua
> @@ -368,4 +368,28 @@ for i = 1, 200 do ch:get() end
>  errinj.set("ERRINJ_IPROTO_TX_DELAY", false)
>  
>  s:drop()
> +
> +--
> +-- gh-3325: do not cancel already sent requests, when a schema
> +-- change is detected.
> +--
> +s = box.schema.create_space('test')
> +pk = s:create_index('pk')
> +s:replace{1, 1}
> +cn = net_box.connect(box.cfg.listen)
> +errinj.set("ERRINJ_WAL_DELAY", true)
> +ok = nil
> +err = nil
> +test_run:cmd('setopt delimiter ";"')
> +f = fiber.create(function()
> +  local str = 'box.space.test:create_index("sk", {parts = {{2, "integer"}}})'
> +  ok, err = pcall(cn.eval, cn, str)
> +end)
> +test_run:cmd('setopt delimiter ""');
> +cn.space.test:get{1}
> +errinj.set("ERRINJ_WAL_DELAY", false)
> +ok, err
> +cn:close()
> +s:drop()
> +
>  box.schema.user.revoke('guest', 'read,write,execute','universe')
> -- 
> 2.14.3 (Apple Git-98)
> 
> 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

      reply	other threads:[~2018-04-06 14:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-06 13:46 Vladislav Shpilevoy
2018-04-06 14:01 ` Konstantin Osipov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180406140135.GC18946@atlas \
    --to=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [tarantool-patches] [PATCH 1/1] netbox: don'\''t cancel pending requests on schema change' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox