* [PATCH 1/1] netbox: don't cancel pending requests on schema change
@ 2018-04-06 13:46 Vladislav Shpilevoy
2018-04-06 14:01 ` [tarantool-patches] " Konstantin Osipov
0 siblings, 1 reply; 2+ messages in thread
From: Vladislav Shpilevoy @ 2018-04-06 13:46 UTC (permalink / raw)
To: tarantool-patches; +Cc: vdavydov.dev
When a schema version change is detected, there is no reason to
cancel and retry already sent requests. They can be already
executed on a server, and their retrying leads to multiple
execution.
A request must be retried only if a server responded with
WRONG_SCHEMA_VERSION error exactly to this request.
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)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [tarantool-patches] [PATCH 1/1] netbox: don't cancel pending requests on schema change
2018-04-06 13:46 [PATCH 1/1] netbox: don't cancel pending requests on schema change Vladislav Shpilevoy
@ 2018-04-06 14:01 ` Konstantin Osipov
0 siblings, 0 replies; 2+ messages in thread
From: Konstantin Osipov @ 2018-04-06 14:01 UTC (permalink / raw)
To: tarantool-patches; +Cc: vdavydov.dev
* 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-04-06 14:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-06 13:46 [PATCH 1/1] netbox: don't cancel pending requests on schema change Vladislav Shpilevoy
2018-04-06 14:01 ` [tarantool-patches] " Konstantin Osipov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox