From: Serge Petrenko <sergepetrenko@tarantool.org> To: vdavydov.dev@gmail.com Cc: tarantool-patches@freelists.org, Serge Petrenko <sergepetrenko@tarantool.org> Subject: [PATCH 2/2] recovery: fix incorrect handling of empty-body requests. Date: Fri, 14 Sep 2018 17:36:41 +0300 [thread overview] Message-ID: <724c197f81170aac18d37d976c835177925959f6.1536934854.git.sergepetrenko@tarantool.org> (raw) In-Reply-To: <cover.1536934854.git.sergepetrenko@tarantool.org> In-Reply-To: <cover.1536934854.git.sergepetrenko@tarantool.org> In some cases no-ops are written to xlog. They have no effect but are needed to bump lsn. Such ops have no body, and empty body requests are not handled in xrow_header_decode(). This leads to recovery errors in special case: when we have a multi-statement transaction containing no-ops written to xlog, upon recovering from such xlog, all data after the no-op end till the start of new transaction will become no-ops body, so, effectively, it will be ignored. Here's example `tarantoolctl cat` output showing this (BODY contains next request data): ``` --- HEADER: lsn: 5 replica_id: 1 type: NOP timestamp: 1536656270.5092 BODY: type: 3 timestamp: 1536656270.5092 lsn: 6 replica_id: 1 --- HEADER: type: 0 ... ``` This patch handles no-ops correctly in xrow_header_decode(). Closes #3678 --- src/box/xrow.c | 6 +++- test/xlog/recover_nop.result | 72 ++++++++++++++++++++++++++++++++++++++++++ test/xlog/recover_nop.test.lua | 28 ++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 test/xlog/recover_nop.result create mode 100644 test/xlog/recover_nop.test.lua diff --git a/src/box/xrow.c b/src/box/xrow.c index 7a35d0db1..99dce5395 100644 --- a/src/box/xrow.c +++ b/src/box/xrow.c @@ -99,6 +99,8 @@ error: if (mp_typeof(**pos) != MP_MAP) goto error; + bool expect_body = true; + uint32_t size = mp_decode_map(pos); for (uint32_t i = 0; i < size; i++) { if (mp_typeof(**pos) != MP_UINT) @@ -110,6 +112,8 @@ error: switch (key) { case IPROTO_REQUEST_TYPE: header->type = mp_decode_uint(pos); + if (header->type == IPROTO_NOP) + expect_body = false; break; case IPROTO_SYNC: header->sync = mp_decode_uint(pos); @@ -135,7 +139,7 @@ error: } } assert(*pos <= end); - if (*pos < end) { + if (*pos < end && expect_body) { const char *body = *pos; if (mp_check(pos, end)) { diag_set(ClientError, ER_INVALID_MSGPACK, "packet body"); diff --git a/test/xlog/recover_nop.result b/test/xlog/recover_nop.result new file mode 100644 index 000000000..e6aad41a7 --- /dev/null +++ b/test/xlog/recover_nop.result @@ -0,0 +1,72 @@ +test_run = require('test_run').new() +--- +... +test_run:cmd('restart server default with cleanup=1') +-- gh-3678 check for correct recovery with nops in transaction. +s = box.schema.space.create('test') +--- +... +_ = s:create_index('pk') +--- +... +s:replace{1,1} +--- +- [1, 1] +... +f = function(old, new) return old end +--- +... +box.begin() s:before_replace(f) s:replace{1,2} s:before_replace(nil, f) s:replace{1,3} box.commit() +--- +... +s:select{} +--- +- - [1, 3] +... +test_run:cmd('restart server default') +xlog = require('xlog') +--- +... +fio = require('fio') +--- +... +box.space.test:select{} +--- +- - [1, 3] +... +xlog_path = fio.pathjoin(box.cfg.wal_dir, string.format('%020d.xlog', 0)) +--- +... +fun, param, state = xlog.pairs(xlog_path) +--- +... +repeat state, row = fun(param, state) until row.HEADER.type == 'NOP' +--- +... +row.BODY == nil +--- +- true +... +state, row = fun(param, state) +--- +... +row.HEADER.lsn +--- +- 6 +... +row.HEADER.replica_id +--- +- 1 +... +row.HEADER.type +--- +- REPLACE +... +row.BODY +--- +- space_id: 512 + tuple: [1, 3] +... +box.space.test:drop() +--- +... diff --git a/test/xlog/recover_nop.test.lua b/test/xlog/recover_nop.test.lua new file mode 100644 index 000000000..de8f8ef42 --- /dev/null +++ b/test/xlog/recover_nop.test.lua @@ -0,0 +1,28 @@ +test_run = require('test_run').new() + +test_run:cmd('restart server default with cleanup=1') +-- gh-3678 check for correct recovery with nops in transaction. +s = box.schema.space.create('test') +_ = s:create_index('pk') +s:replace{1,1} +f = function(old, new) return old end +box.begin() s:before_replace(f) s:replace{1,2} s:before_replace(nil, f) s:replace{1,3} box.commit() +s:select{} + +test_run:cmd('restart server default') + +xlog = require('xlog') +fio = require('fio') + +box.space.test:select{} +xlog_path = fio.pathjoin(box.cfg.wal_dir, string.format('%020d.xlog', 0)) +fun, param, state = xlog.pairs(xlog_path) +repeat state, row = fun(param, state) until row.HEADER.type == 'NOP' +row.BODY == nil +state, row = fun(param, state) +row.HEADER.lsn +row.HEADER.replica_id +row.HEADER.type +row.BODY + +box.space.test:drop() -- 2.15.2 (Apple Git-101.1)
next prev parent reply other threads:[~2018-09-14 14:36 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-14 14:36 [PATCH 0/2] fix bodiless requests handling Serge Petrenko 2018-09-14 14:36 ` [PATCH 1/2] tarantoolctl: fix cat and play for empty body requests Serge Petrenko 2018-09-14 15:37 ` Vladimir Davydov 2018-09-14 14:36 ` Serge Petrenko [this message] 2018-09-14 16:27 ` [PATCH 2/2] recovery: fix incorrect handling of empty-body requests Vladimir Davydov
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=724c197f81170aac18d37d976c835177925959f6.1536934854.git.sergepetrenko@tarantool.org \ --to=sergepetrenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 2/2] recovery: fix incorrect handling of empty-body requests.' \ /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