[tarantool-patches] [wal 1/1] wal: Update request header after sequence update

[tarantool-patches] [wal 1/1] wal: Update request header after sequence update

[Ilya Markov]

When tuple in insert/replace request has NULL value
in the field incremented by sequence,
request body is changed, NULL is replaced by value taken from
sequence.
But request header is not updated.
So Redo log, which takes body from header if header exists,
writes the old version of request to wal.

Fixed this with updating header value after handling the sequence.

Closes #3247
---
branch gh-3247-wrong-sequence-primary-index

 src/box/request.c              |  7 +++++
 test/replication/misc.result   | 66 ++++++++++++++++++++++++++++++++++++++++++
 test/replication/misc.test.lua | 21 ++++++++++++++
 3 files changed, 94 insertions(+)

diff --git a/src/box/request.c b/src/box/request.c
index 646da42..0b3c5fb 100644
--- a/src/box/request.c
+++ b/src/box/request.c
@@ -195,5 +195,12 @@ request_handle_sequence(struct request *request, struct space *space)
 		if (likely(mp_read_int64(&key, &value) == 0))
 			return sequence_update(seq, value);
 	}
+	/*
+	 * As the request body was changed, we have to update body in header.
+	 */
+	if (request->header != NULL) {
+		request->header->bodycnt =
+			xrow_encode_dml(request, request->header->body);
+	}
 	return 0;
 }
diff --git a/test/replication/misc.result b/test/replication/misc.result
index 879c7fe..3d1089d 100644
--- a/test/replication/misc.result
+++ b/test/replication/misc.result
@@ -139,6 +139,72 @@ test_run:cmd("switch default")
 ---
 - true
 ...
+-- gh-3247 wrong sequence replication
+test_run:cmd("switch autobootstrap1")
+---
+- true
+...
+net_box = require('net.box')
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+_ = box.schema.space.create("space1", {engine=engine})
+---
+...
+_ = box.schema.sequence.create('seq')
+---
+...
+_ = box.space.space1:create_index('primary', {sequence='seq'} )
+---
+...
+_ = box.space.space1:create_index('idx', { type = 'tree', unique = false, parts = {2, 'number'} })
+---
+...
+box.schema.user.grant('guest', "read,write", "space", 'space1')
+---
+...
+box.schema.user.grant('guest', "read,write", "sequence", 'seq')
+---
+...
+c = net_box.connect(box.cfg.listen)
+---
+...
+c.space.space1:insert{box.NULL, "data"}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected number'
+...
+c.space.space1:insert{box.NULL, 1, "data"}
+---
+- [2, 1, 'data']
+...
+box.space.space1:select{}
+---
+- - [2, 1, 'data']
+...
+test_run:cmd("switch autobootstrap2")
+---
+- true
+...
+box.space.space1:select{}
+---
+- - [2, 1, 'data']
+...
+test_run:cmd("switch autobootstrap1")
+---
+- true
+...
+box.space.space1:drop()
+---
+...
+box.sequence.seq:drop()
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
 test_run:drop_cluster(SERVERS)
 ---
 ...
diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua
index 8752182..b00154e 100644
--- a/test/replication/misc.test.lua
+++ b/test/replication/misc.test.lua
@@ -56,6 +56,27 @@ end ;
 test_run:cmd("setopt delimiter ''");
 test_timeout()
 test_run:cmd("switch default")
+-- gh-3247 wrong sequence replication
+test_run:cmd("switch autobootstrap1")
+net_box = require('net.box')
+engine = test_run:get_cfg('engine')
+_ = box.schema.space.create("space1", {engine=engine})
+_ = box.schema.sequence.create('seq')
+_ = box.space.space1:create_index('primary', {sequence='seq'} )
+_ = box.space.space1:create_index('idx', { type = 'tree', unique = false, parts = {2, 'number'} })
+box.schema.user.grant('guest', "read,write", "space", 'space1')
+box.schema.user.grant('guest', "read,write", "sequence", 'seq')
+c = net_box.connect(box.cfg.listen)
+c.space.space1:insert{box.NULL, "data"}
+c.space.space1:insert{box.NULL, 1, "data"}
+box.space.space1:select{}
+test_run:cmd("switch autobootstrap2")
+box.space.space1:select{}
+test_run:cmd("switch autobootstrap1")
+box.space.space1:drop()
+box.sequence.seq:drop()
+
+test_run:cmd("switch default")
 test_run:drop_cluster(SERVERS)
 
 box.schema.user.revoke('guest', 'replication')
-- 
2.7.4

[tarantool-patches] Re: [wal 1/1] wal: Update request header after sequence update

[Georgy Kirichenko]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

Seems good for me
Thanks for investigation

On Tuesday, April 17, 2018 10:39:16 AM MSK Ilya Markov wrote:
> When tuple in insert/replace request has NULL value
> in the field incremented by sequence,
> request body is changed, NULL is replaced by value taken from
> sequence.
> But request header is not updated.
> So Redo log, which takes body from header if header exists,
> writes the old version of request to wal.
> 
> Fixed this with updating header value after handling the sequence.
> 
> Closes #3247
> ---
> branch gh-3247-wrong-sequence-primary-index

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [tarantool-patches] [wal 1/1] wal: Update request header after sequence update

[Vladimir Davydov]

On Tue, Apr 17, 2018 at 10:39:16AM +0300, Ilya Markov wrote:
> When tuple in insert/replace request has NULL value
> in the field incremented by sequence,
> request body is changed, NULL is replaced by value taken from
> sequence.
> But request header is not updated.
> So Redo log, which takes body from header if header exists,
> writes the old version of request to wal.
> 
> Fixed this with updating header value after handling the sequence.
> 
> Closes #3247
> ---
> branch gh-3247-wrong-sequence-primary-index
> 
>  src/box/request.c              |  7 +++++
>  test/replication/misc.result   | 66 ++++++++++++++++++++++++++++++++++++++++++
>  test/replication/misc.test.lua | 21 ++++++++++++++
>  3 files changed, 94 insertions(+)

Pushed to 1.9.