[Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Serge Petrenko via Tarantool-patches]

Every error that happens during master processes a join or subscribe
request is sent to the replica for better diagnostics.

This could lead to the following situation with the TimedOut error:
it could be written on top of a half-written row and make the replica stop
replication with ER_INVALID_MSGPACK error. The error is unrecoverable
and the only way to resume replication after it happens is to reset
box.cfg.replication.

Here's what happened:

1) Replica is under heavy load, meaning it's event loop is occupied by
   some fiber not yielding control to others.

2) applier and other fibers aren't scheduled while the event loop is
   blocked. This means applier doesn't send heartbeat messages to the
   master and doesn't read any data coming from the master.

3) The unread master's data piles up. First in replica's receive buffer, then
   in master's send buffer.

4) Once master's send buffer is full, the corresponding socket stops
   being writeable and the relay yields waiting for the socket to become
   writeable again. The send buffer might contain a partially written
   row by now.

5) Replication timeout happens on master, because it hasn't heard from
   replica for a while. An exception is raised, and the exception is
   pushed to the replica's socket. Now two situations are possible:

  a) the socket becomes writeable by the time exception is raised.
     In this case the exception is logged to the buffer right after
     a partially written row. Once replica receives the half-written
     row with an exception logged on top, it errors with
     ER_INVALID_MSGPACK. Replication is broken.

  b) the socket isn't writeable still (the most probable scenario)
     The exception isn't logged to the socket and the connection is
     closed. Replica eventually receives a partially-written row and
     retries connection to the master normally.

In order to prevent case a) from happening, let's not push TimedOut
errors to the socket at all. They're the only errors that could be
raised while a row is being written, i.e. the only errors that could
lead to the situation described in 5a.

Closes #4040
---
https://github.com/tarantool/tarantool/issues/4040
https://github.com/tarantool/tarantool/compare/sp/gh-4040-invalid-msgpack

 .../unreleased/gh-4040-invalid-msgpack.md     |   4 +
 src/box/applier.cc                            |   2 +
 src/box/iproto.cc                             |   6 +
 src/box/xrow.c                                |   9 +
 src/lib/core/errinj.h                         |   3 +
 test/box/errinj.result                        |   5 +-
 test/replication/errinj.result                |  12 +-
 test/replication/errinj.test.lua              |   8 +-
 .../gh-4040-invalid-msgpack.result            | 180 ++++++++++++++++++
 .../gh-4040-invalid-msgpack.test.lua          |  74 +++++++
 test/replication/suite.cfg                    |   1 +
 test/replication/suite.ini                    |   2 +-
 12 files changed, 296 insertions(+), 10 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4040-invalid-msgpack.md
 create mode 100644 test/replication/gh-4040-invalid-msgpack.result
 create mode 100644 test/replication/gh-4040-invalid-msgpack.test.lua

diff --git a/changelogs/unreleased/gh-4040-invalid-msgpack.md b/changelogs/unreleased/gh-4040-invalid-msgpack.md
new file mode 100644
index 000000000..1a89fefae
--- /dev/null
+++ b/changelogs/unreleased/gh-4040-invalid-msgpack.md
@@ -0,0 +1,4 @@
+## bugfix/replication
+
+* Fix replication stopping occasionally with `ER_INVALID_MSGPACK` when replica
+  is under high load (gh-4040).
diff --git a/src/box/applier.cc b/src/box/applier.cc
index 07fe7f5c7..a9b6cacbc 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -635,6 +635,8 @@ applier_read_tx_row(struct applier *applier, double timeout)
 
 	struct xrow_header *row = &tx_row->row;
 
+	ERROR_INJECT_YIELD(ERRINJ_APPLIER_READ_TX_ROW_DELAY);
+
 	coio_read_xrow_timeout_xc(coio, ibuf, row, timeout);
 
 	applier->lag = ev_now(loop()) - row->tm;
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index ac1cb6081..3ed641eea 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1840,6 +1840,12 @@ tx_process_replication(struct cmsg *m)
 		}
 	} catch (SocketError *e) {
 		return; /* don't write error response to prevent SIGPIPE */
+	} catch (TimedOut *e) {
+		 /*
+		  * In case of a timeout the error could come after a partially
+		  * written row. Do not push it on top.
+		  */
+		return;
 	} catch (Exception *e) {
 		iproto_write_error(con->input.fd, e, ::schema_version,
 				   msg->header.sync);
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 16cb2484c..fae9861a5 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -43,6 +43,7 @@
 #include "scramble.h"
 #include "iproto_constants.h"
 #include "mpstream/mpstream.h"
+#include "errinj.h"
 
 static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA < 0x7f &&
 	      IPROTO_SQL_INFO < 0x7f, "encoded IPROTO_BODY keys must fit into "\
@@ -549,6 +550,12 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
 
 	size_t region_svp = region_used(region);
 	mpstream_iproto_encode_error(&stream, e);
+	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
+				    ERRINJ_INT);
+	if (inj != NULL && inj->iparam > 0) {
+		char garbage[inj->iparam];
+		mpstream_encode_strn(&stream, garbage, inj->iparam);
+	}
 	mpstream_flush(&stream);
 	if (is_error)
 		goto cleanup;
@@ -564,6 +571,8 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
 			     schema_version, payload_size);
 
 	ssize_t unused;
+
+	ERROR_INJECT_YIELD(ERRINJ_IPROTO_WRITE_ERROR_DELAY);
 	unused = write(fd, header, sizeof(header));
 	unused = write(fd, payload, payload_size);
 	(void) unused;
diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
index 359174b16..0d8ec967d 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -152,6 +152,9 @@ struct errinj {
 	_(ERRINJ_STDIN_ISATTY, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_SNAP_COMMIT_FAIL, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_IPROTO_SINGLE_THREAD_STAT, ERRINJ_INT, {.iparam = -1}) \
+	_(ERRINJ_IPROTO_WRITE_ERROR_DELAY, ERRINJ_BOOL, {.bparam = false})\
+	_(ERRINJ_IPROTO_WRITE_ERROR_LARGE, ERRINJ_INT, {.iparam = -1})\
+	_(ERRINJ_APPLIER_READ_TX_ROW_DELAY, ERRINJ_BOOL, {.bparam = false})\
 
 ENUM0(errinj_id, ERRINJ_LIST);
 extern struct errinj errinjs[];
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 43daf5f0f..e5193a3d3 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -43,7 +43,8 @@ end
 ...
 evals
 ---
-- - ERRINJ_APPLIER_SLOW_ACK: false
+- - ERRINJ_APPLIER_READ_TX_ROW_DELAY: false
+  - ERRINJ_APPLIER_SLOW_ACK: false
   - ERRINJ_AUTO_UPGRADE: false
   - ERRINJ_BUILD_INDEX: -1
   - ERRINJ_BUILD_INDEX_DELAY: false
@@ -59,6 +60,8 @@ evals
   - ERRINJ_INDEX_RESERVE: false
   - ERRINJ_IPROTO_SINGLE_THREAD_STAT: -1
   - ERRINJ_IPROTO_TX_DELAY: false
+  - ERRINJ_IPROTO_WRITE_ERROR_DELAY: false
+  - ERRINJ_IPROTO_WRITE_ERROR_LARGE: -1
   - ERRINJ_LOG_ROTATE: false
   - ERRINJ_MEMTX_DELAY_GC: false
   - ERRINJ_PORT_DUMP: false
diff --git a/test/replication/errinj.result b/test/replication/errinj.result
index f04a38c45..9d13f6aa7 100644
--- a/test/replication/errinj.result
+++ b/test/replication/errinj.result
@@ -275,8 +275,9 @@ test_run:cmd("switch replica")
 fiber = require'fiber'
 ---
 ...
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='unexpected EOF'})
 ---
+- true
 ...
 test_run:cmd("switch default")
 ---
@@ -312,8 +313,9 @@ box.info.replication[1].upstream.lag < 1
 - true
 ...
 -- wait for ack timeout
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='unexpected EOF'})
 ---
+- true
 ...
 test_run:cmd("switch default")
 ---
@@ -450,8 +452,9 @@ test_run:cmd("switch replica_timeout")
 -- due to infinite read timeout connection never breaks,
 -- replica shows state 'follow' so old behaviour hangs
 -- here in infinite loop.
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='timed out'})
 ---
+- true
 ...
 test_run:cmd("switch default")
 ---
@@ -530,8 +533,9 @@ test_run:cmd("switch replica_timeout")
 fiber = require('fiber')
 ---
 ...
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='timed out'})
 ---
+- true
 ...
 test_run:cmd("stop server default")
 ---
diff --git a/test/replication/errinj.test.lua b/test/replication/errinj.test.lua
index 53637e248..19234ab35 100644
--- a/test/replication/errinj.test.lua
+++ b/test/replication/errinj.test.lua
@@ -118,7 +118,7 @@ box.cfg{replication_timeout = 0.0001}
 test_run:cmd("start server replica")
 test_run:cmd("switch replica")
 fiber = require'fiber'
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='unexpected EOF'})
 
 test_run:cmd("switch default")
 -- Disable heartbeat messages on the master so as not
@@ -132,7 +132,7 @@ box.info.replication[1].upstream.status
 box.info.replication[1].upstream.lag > 0
 box.info.replication[1].upstream.lag < 1
 -- wait for ack timeout
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='unexpected EOF'})
 
 test_run:cmd("switch default")
 errinj.set("ERRINJ_RELAY_REPORT_INTERVAL", 0)
@@ -188,7 +188,7 @@ test_run:cmd("switch replica_timeout")
 -- due to infinite read timeout connection never breaks,
 -- replica shows state 'follow' so old behaviour hangs
 -- here in infinite loop.
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='timed out'})
 
 test_run:cmd("switch default")
 test_run:cmd("stop server replica_timeout")
@@ -221,7 +221,7 @@ for i = 0, 9999 do box.space.test:replace({i, 4, 5, 'test'}) end
 test_run:cmd("start server replica_timeout with args='0.00001 0.5'")
 test_run:cmd("switch replica_timeout")
 fiber = require('fiber')
-while box.info.replication[1].upstream.message ~= 'timed out' do fiber.sleep(0.0001) end
+test_run:wait_upstream(1, {status='disconnected', message_re='timed out'})
 
 test_run:cmd("stop server default")
 test_run:cmd("deploy server default")
diff --git a/test/replication/gh-4040-invalid-msgpack.result b/test/replication/gh-4040-invalid-msgpack.result
new file mode 100644
index 000000000..4348ebd90
--- /dev/null
+++ b/test/replication/gh-4040-invalid-msgpack.result
@@ -0,0 +1,180 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+--
+-- gh-4040. ER_INVALID_MSGPACK on a replica when master's relay times out after
+-- not being able to write a full row to the socket.
+--
+test_run:cmd('create server master with script="replication/master1.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('create server replica with rpl_master=master,\
+                                         script="replication/replica_timeout.lua"')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('start server master')
+ | ---
+ | - true
+ | ...
+test_run:switch('master')
+ | ---
+ | - true
+ | ...
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+box.cfg{replication_timeout=0.5}
+ | ---
+ | ...
+_ = box.schema.space.create('test')
+ | ---
+ | ...
+_ = box.space.test:create_index('pk')
+ | ---
+ | ...
+
+test_run:cmd('start server replica with args="1000"')
+ | ---
+ | - true
+ | ...
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+
+box.cfg{log_level=6}
+ | ---
+ | ...
+sign = box.info.signature
+ | ---
+ | ...
+box.error.injection.set('ERRINJ_APPLIER_READ_TX_ROW_DELAY', true)
+ | ---
+ | - ok
+ | ...
+test_run:switch('master')
+ | ---
+ | - true
+ | ...
+
+-- Find the send buffer size. Testing uses Unix domain sockets. Create such a
+-- socket and assume relay's socket has the same parameters.
+socket = require('socket')
+ | ---
+ | ...
+soc = socket('AF_UNIX', 'SOCK_STREAM', 0)
+ | ---
+ | ...
+bufsize = soc:getsockopt('SOL_SOCKET', 'SO_SNDBUF')
+ | ---
+ | ...
+require('log').info("SO_SNDBUF size is %d", bufsize)
+ | ---
+ | ...
+-- Master shouldn't try to write the error while the socket isn't writeable.
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', true)
+ | ---
+ | - ok
+ | ...
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', bufsize)
+ | ---
+ | - ok
+ | ...
+-- Generate enough data to fill the sendbuf.
+-- This will make the relay yield in the middle of writing a row waiting for the
+-- socket to become writeable.
+tbl = {1}
+ | ---
+ | ...
+filler = string.rep('b', 100)
+ | ---
+ | ...
+for i = 2, 0.7 * bufsize / 100 do\
+    tbl[i] = filler\
+end
+ | ---
+ | ...
+for i = 1,10 do\
+    tbl[1] = i\
+    box.space.test:replace(tbl)\
+end
+ | ---
+ | ...
+
+-- Wait for the timeout to happen. The relay's send buffer should full by now
+-- and contain a half-written row.
+test_run:wait_downstream(2, {status='stopped'})
+ | ---
+ | - true
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+-- Wait until replica starts receiving the data.
+-- This will make master's socket writeable again.
+box.error.injection.set('ERRINJ_APPLIER_READ_TX_ROW_DELAY', false)
+ | ---
+ | - ok
+ | ...
+test_run:wait_cond(function() return box.info.signature > sign end)
+ | ---
+ | - true
+ | ...
+
+test_run:switch('master')
+ | ---
+ | - true
+ | ...
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', false)
+ | ---
+ | - ok
+ | ...
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', -1)
+ | ---
+ | - ok
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+
+-- There shouldn't be any errors other than the connection reset.
+test_run:wait_upstream(1, {status='disconnected', message_re='unexpected EOF'})
+ | ---
+ | - true
+ | ...
+assert(test_run:grep_log('replica', 'ER_INVALID_MSGPACK') == nil)
+ | ---
+ | - true
+ | ...
+
+-- Cleanup.
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server master')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server master')
+ | ---
+ | - true
+ | ...
+
diff --git a/test/replication/gh-4040-invalid-msgpack.test.lua b/test/replication/gh-4040-invalid-msgpack.test.lua
new file mode 100644
index 000000000..0e51affac
--- /dev/null
+++ b/test/replication/gh-4040-invalid-msgpack.test.lua
@@ -0,0 +1,74 @@
+test_run = require('test_run').new()
+
+--
+-- gh-4040. ER_INVALID_MSGPACK on a replica when master's relay times out after
+-- not being able to write a full row to the socket.
+--
+test_run:cmd('create server master with script="replication/master1.lua"')
+test_run:cmd('create server replica with rpl_master=master,\
+                                         script="replication/replica_timeout.lua"')
+
+test_run:cmd('start server master')
+test_run:switch('master')
+box.schema.user.grant('guest', 'replication')
+box.cfg{replication_timeout=0.5}
+_ = box.schema.space.create('test')
+_ = box.space.test:create_index('pk')
+
+test_run:cmd('start server replica with args="1000"')
+test_run:switch('replica')
+
+box.cfg{log_level=6}
+sign = box.info.signature
+box.error.injection.set('ERRINJ_APPLIER_READ_TX_ROW_DELAY', true)
+test_run:switch('master')
+
+-- Find the send buffer size. Testing uses Unix domain sockets. Create such a
+-- socket and assume relay's socket has the same parameters.
+socket = require('socket')
+soc = socket('AF_UNIX', 'SOCK_STREAM', 0)
+bufsize = soc:getsockopt('SOL_SOCKET', 'SO_SNDBUF')
+require('log').info("SO_SNDBUF size is %d", bufsize)
+-- Master shouldn't try to write the error while the socket isn't writeable.
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', true)
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', bufsize)
+-- Generate enough data to fill the sendbuf.
+-- This will make the relay yield in the middle of writing a row waiting for the
+-- socket to become writeable.
+tbl = {1}
+filler = string.rep('b', 100)
+for i = 2, 0.7 * bufsize / 100 do\
+    tbl[i] = filler\
+end
+for i = 1,10 do\
+    tbl[1] = i\
+    box.space.test:replace(tbl)\
+end
+
+-- Wait for the timeout to happen. The relay's send buffer should full by now
+-- and contain a half-written row.
+test_run:wait_downstream(2, {status='stopped'})
+
+test_run:switch('replica')
+-- Wait until replica starts receiving the data.
+-- This will make master's socket writeable again.
+box.error.injection.set('ERRINJ_APPLIER_READ_TX_ROW_DELAY', false)
+test_run:wait_cond(function() return box.info.signature > sign end)
+
+test_run:switch('master')
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', false)
+box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', -1)
+
+test_run:switch('replica')
+
+-- There shouldn't be any errors other than the connection reset.
+test_run:wait_upstream(1, {status='disconnected', message_re='unexpected EOF'})
+assert(test_run:grep_log('replica', 'ER_INVALID_MSGPACK') == nil)
+
+-- Cleanup.
+test_run:switch('default')
+test_run:cmd('stop server replica')
+test_run:cmd('stop server master')
+test_run:cmd('delete server replica')
+test_run:cmd('delete server master')
+
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index 69f2f3511..a51a2d51a 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -33,6 +33,7 @@
     "on_schema_init.test.lua": {},
     "long_row_timeout.test.lua": {},
     "join_without_snap.test.lua": {},
+    "gh-4040-invalid-msgpack.test.lua": {},
     "gh-4114-local-space-replication.test.lua": {},
     "gh-4402-info-errno.test.lua": {},
     "gh-4605-empty-password.test.lua": {},
diff --git a/test/replication/suite.ini b/test/replication/suite.ini
index 6ae041d12..18981996d 100644
--- a/test/replication/suite.ini
+++ b/test/replication/suite.ini
@@ -3,7 +3,7 @@ core = tarantool
 script =  master.lua
 description = tarantool/box, replication
 disabled = consistent.test.lua
-release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua hang_on_synchro_fail.test.lua anon_register_gap.test.lua gh-5213-qsync-applier-order.test.lua gh-5213-qsync-applier-order-3.test.lua gh-6027-applier-error-show.test.lua gh-6032-promote-wal-write.test.lua gh-6057-qsync-confirm-async-no-wal.test.lua gh-5447-downstream-lag.test.lua
+release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua hang_on_synchro_fail.test.lua anon_register_gap.test.lua gh-5213-qsync-applier-order.test.lua gh-5213-qsync-applier-order-3.test.lua gh-6027-applier-error-show.test.lua gh-6032-promote-wal-write.test.lua gh-6057-qsync-confirm-async-no-wal.test.lua gh-5447-downstream-lag.test.lua gh-4040-invalid-msgpack.test.lua
 config = suite.cfg
 lua_libs = lua/fast_replica.lua lua/rlimit.lua
 use_unix_sockets = True
-- 
2.30.1 (Apple Git-130)

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Cyrill Gorcunov via Tarantool-patches]

On Fri, Jul 09, 2021 at 10:40:48AM +0300, Serge Petrenko wrote:
> @@ -549,6 +550,12 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
>  
>  	size_t region_svp = region_used(region);
>  	mpstream_iproto_encode_error(&stream, e);
> +	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
> +				    ERRINJ_INT);
> +	if (inj != NULL && inj->iparam > 0) {
> +		char garbage[inj->iparam];

Serge, I didn't read the patch yet but this moment is very dubious.
While standart allows to allocate dynamic stack here it may lead
to unpredicted effects. I would suggest to use malloc if this
won't cause significant rework, hm? Surely this can be done on
top of the patch (actually you did a huge amount of great work
investigating this issue, thanks!!!)

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Serge Petrenko via Tarantool-patches]

09.07.2021 10:59, Cyrill Gorcunov пишет:
> On Fri, Jul 09, 2021 at 10:40:48AM +0300, Serge Petrenko wrote:
>> @@ -549,6 +550,12 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
>>   
>>   	size_t region_svp = region_used(region);
>>   	mpstream_iproto_encode_error(&stream, e);
>> +	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
>> +				    ERRINJ_INT);
>> +	if (inj != NULL && inj->iparam > 0) {
>> +		char garbage[inj->iparam];
> Serge, I didn't read the patch yet but this moment is very dubious.
> While standart allows to allocate dynamic stack here it may lead
> to unpredicted effects. I would suggest to use malloc if this
> won't cause significant rework, hm? Surely this can be done on
> top of the patch (actually you did a huge amount of great work
> investigating this issue, thanks!!!)

Thanks! No problem, will rework.
It might be better to not allocate any string at all.
I can simply allocate the needed buffer on the region (where the msgpack 
will be encoded),
and encode the string header, and leave the buffer with whatever 
contents it has
(it'll probably be filled with 0x50 bytes, it's the slab poison symbol).

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Cyrill Gorcunov via Tarantool-patches]

On Fri, Jul 09, 2021 at 11:30:20AM +0300, Serge Petrenko wrote:
> 
> Thanks! No problem, will rework.
> It might be better to not allocate any string at all.
> I can simply allocate the needed buffer on the region (where the msgpack
> will be encoded), and encode the string header, and leave the
> buffer with whatever contents it has (it'll probably be filled
> with 0x50 bytes, it's the slab poison symbol).

Cool, good idea!

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Serge Petrenko via Tarantool-patches]

09.07.2021 13:41, Cyrill Gorcunov пишет:
> On Fri, Jul 09, 2021 at 11:30:20AM +0300, Serge Petrenko wrote:
>> Thanks! No problem, will rework.
>> It might be better to not allocate any string at all.
>> I can simply allocate the needed buffer on the region (where the msgpack
>> will be encoded), and encode the string header, and leave the
>> buffer with whatever contents it has (it'll probably be filled
>> with 0x50 bytes, it's the slab poison symbol).
>>
>> Cool, good idea!

I've found an even better solution:

We don't care for the actual contents of the error, so
it's fine to fill the buffer with whatever junk we come up with.
(That's because the error is meant to fill the remaining buffer
space after a partially written row).
'E' is for error injection, since we have 'P' for slab poison.

I've force-pushed the following change:

====================

diff --git a/src/box/xrow.c b/src/box/xrow.c
index fae9861a5..c4647c6cb 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -552,10 +552,8 @@ iproto_write_error(int fd, const struct error *e, 
uint32_t schema_version,
         mpstream_iproto_encode_error(&stream, e);
         struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
                                     ERRINJ_INT);
-       if (inj != NULL && inj->iparam > 0) {
-               char garbage[inj->iparam];
-               mpstream_encode_strn(&stream, garbage, inj->iparam);
-       }
+       if (inj != NULL && inj->iparam > 0)
+               mpstream_memset(&stream, 'E', inj->iparam);
         mpstream_flush(&stream);
         if (is_error)
                 goto cleanup;

=====================

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Cyrill Gorcunov via Tarantool-patches]

On Mon, Jul 12, 2021 at 01:14:09PM +0300, Serge Petrenko wrote:
> 'E' is for error injection, since we have 'P' for slab poison.
> 
> I've force-pushed the following change:

Great! Ack.

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Good job on the patch! Especially on the
explanation in the commit message.

> diff --git a/src/box/xrow.c b/src/box/xrow.c
> index 16cb2484c..c4647c6cb 100644
> --- a/src/box/xrow.c
> +++ b/src/box/xrow.c
> @@ -549,6 +550,10 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
>  
>  	size_t region_svp = region_used(region);
>  	mpstream_iproto_encode_error(&stream, e);
> +	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
> +				    ERRINJ_INT);
> +	if (inj != NULL && inj->iparam > 0)
> +		mpstream_memset(&stream, 'E', inj->iparam);

Error injections are supposed to simulate something what can happen
in reality. Otherwise they might test an impossible case, like here -
the error object can't be corrupted. If I understood correctly, this
way you are trying to prove that iproto_write_error() is not called
at all. Because if it is, you break the data and see it in the test.
You could make it panic() with the same result I suppose. It is one way.
Another way would be to simulate exactly the happened case - fill the
buffer with an incomplete xrow.

You tried to do it, from what I see in the test. You create a big
tuple which should fill the buffer.

But still it didn't help somewhy and you added the invalid error
object, right?

I managed to remove this injection and still make the test fail 100%
without your patch. I also think I found an explanation of why the
corrupted error object was needed before.

See my comment in the test.

> diff --git a/test/replication/gh-4040-invalid-msgpack.result b/test/replication/gh-4040-invalid-msgpack.result
> new file mode 100644
> index 000000000..4348ebd90
> --- /dev/null
> +++ b/test/replication/gh-4040-invalid-msgpack.result
> @@ -0,0 +1,180 @@

<...>

> +-- Generate enough data to fill the sendbuf.
> +-- This will make the relay yield in the middle of writing a row waiting for the
> +-- socket to become writeable.
> +tbl = {1}
> + | ---
> + | ...
> +filler = string.rep('b', 100)
> + | ---
> + | ...
> +for i = 2, 0.7 * bufsize / 100 do\
> +    tbl[i] = filler\

The reason why you needed the corrupted error object is that the
tuples are not big enough.

Consider my machine (I think you should have the same values). I
have bufsize = 8192 (8KB). Now consider the tuples you create: each
is < 6KB. Each tuple even with all the MessagePack meta fits into
the buffer alone.

Relay writes the xrows using writev(). On my machine this is what I
see when I print its results on your tuples:

	Written 5744
	Written 5744

So the xrows are written one by one, not in parts. I don't know why.
Probably in the kernel it does not accept even a part of the buffer
if sees it does not fit. So you never had in this test a partially
written xrow.

I changed it slightly:

	-for i = 2, 0.7 * bufsize / 100 do\
	+for i = 2, bufsize / 100 + 1 do\

And removed your ERRINJ_IPROTO_WRITE_ERROR_LARGE injection. Now
each tuple is > 8KB. The kernel is forced to write it in parts.
This is what I see:

	Written 8192
	Written 102

The total size of one xrow is > 8KB. The kernel allows to write a
part of the tuple, and now any error, even a valid one, will corrupt
the buffer. That happens if I revert your timeout error ignorance.

What I don't get is why did you have 0.7 coefficient in the loop?

My full diff:

====================
diff --git a/src/box/xrow.c b/src/box/xrow.c
index c4647c6cb..db72437a9 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -550,10 +550,6 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
 
 	size_t region_svp = region_used(region);
 	mpstream_iproto_encode_error(&stream, e);
-	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
-				    ERRINJ_INT);
-	if (inj != NULL && inj->iparam > 0)
-		mpstream_memset(&stream, 'E', inj->iparam);
 	mpstream_flush(&stream);
 	if (is_error)
 		goto cleanup;
diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
index 0d8ec967d..e49242807 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -153,7 +153,6 @@ struct errinj {
 	_(ERRINJ_SNAP_COMMIT_FAIL, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_IPROTO_SINGLE_THREAD_STAT, ERRINJ_INT, {.iparam = -1}) \
 	_(ERRINJ_IPROTO_WRITE_ERROR_DELAY, ERRINJ_BOOL, {.bparam = false})\
-	_(ERRINJ_IPROTO_WRITE_ERROR_LARGE, ERRINJ_INT, {.iparam = -1})\
 	_(ERRINJ_APPLIER_READ_TX_ROW_DELAY, ERRINJ_BOOL, {.bparam = false})\
 
 ENUM0(errinj_id, ERRINJ_LIST);
diff --git a/test/box/errinj.result b/test/box/errinj.result
index e5193a3d3..44f86a54e 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -61,7 +61,6 @@ evals
   - ERRINJ_IPROTO_SINGLE_THREAD_STAT: -1
   - ERRINJ_IPROTO_TX_DELAY: false
   - ERRINJ_IPROTO_WRITE_ERROR_DELAY: false
-  - ERRINJ_IPROTO_WRITE_ERROR_LARGE: -1
   - ERRINJ_LOG_ROTATE: false
   - ERRINJ_MEMTX_DELAY_GC: false
   - ERRINJ_PORT_DUMP: false
diff --git a/test/replication/gh-4040-invalid-msgpack.result b/test/replication/gh-4040-invalid-msgpack.result
index 4348ebd90..def47ed3c 100644
--- a/test/replication/gh-4040-invalid-msgpack.result
+++ b/test/replication/gh-4040-invalid-msgpack.result
@@ -47,9 +47,6 @@ test_run:switch('replica')
  | - true
  | ...
 
-box.cfg{log_level=6}
- | ---
- | ...
 sign = box.info.signature
  | ---
  | ...
@@ -81,10 +78,6 @@ box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', true)
  | ---
  | - ok
  | ...
-box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', bufsize)
- | ---
- | - ok
- | ...
 -- Generate enough data to fill the sendbuf.
 -- This will make the relay yield in the middle of writing a row waiting for the
 -- socket to become writeable.
@@ -94,7 +87,7 @@ tbl = {1}
 filler = string.rep('b', 100)
  | ---
  | ...
-for i = 2, 0.7 * bufsize / 100 do\
+for i = 2, bufsize / 100 + 1 do\
     tbl[i] = filler\
 end
  | ---
@@ -136,10 +129,6 @@ box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', false)
  | ---
  | - ok
  | ...
-box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', -1)
- | ---
- | - ok
- | ...
 
 test_run:switch('replica')
  | ---
diff --git a/test/replication/gh-4040-invalid-msgpack.test.lua b/test/replication/gh-4040-invalid-msgpack.test.lua
index 0e51affac..f7f1c3953 100644
--- a/test/replication/gh-4040-invalid-msgpack.test.lua
+++ b/test/replication/gh-4040-invalid-msgpack.test.lua
@@ -18,7 +18,6 @@ _ = box.space.test:create_index('pk')
 test_run:cmd('start server replica with args="1000"')
 test_run:switch('replica')
 
-box.cfg{log_level=6}
 sign = box.info.signature
 box.error.injection.set('ERRINJ_APPLIER_READ_TX_ROW_DELAY', true)
 test_run:switch('master')
@@ -31,13 +30,12 @@ bufsize = soc:getsockopt('SOL_SOCKET', 'SO_SNDBUF')
 require('log').info("SO_SNDBUF size is %d", bufsize)
 -- Master shouldn't try to write the error while the socket isn't writeable.
 box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', true)
-box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', bufsize)
 -- Generate enough data to fill the sendbuf.
 -- This will make the relay yield in the middle of writing a row waiting for the
 -- socket to become writeable.
 tbl = {1}
 filler = string.rep('b', 100)
-for i = 2, 0.7 * bufsize / 100 do\
+for i = 2, bufsize / 100 + 1 do\
     tbl[i] = filler\
 end
 for i = 1,10 do\
@@ -57,7 +55,6 @@ test_run:wait_cond(function() return box.info.signature > sign end)
 
 test_run:switch('master')
 box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', false)
-box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', -1)
 
 test_run:switch('replica')
 

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Serge Petrenko via Tarantool-patches]

14.07.2021 00:49, Vladislav Shpilevoy пишет:
> Hi! Good job on the patch! Especially on the
> explanation in the commit message.

Hi! Thanks for the review!

TLDR: applied your diff.

Please find my answers below.

>
>> diff --git a/src/box/xrow.c b/src/box/xrow.c
>> index 16cb2484c..c4647c6cb 100644
>> --- a/src/box/xrow.c
>> +++ b/src/box/xrow.c
>> @@ -549,6 +550,10 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
>>   
>>   	size_t region_svp = region_used(region);
>>   	mpstream_iproto_encode_error(&stream, e);
>> +	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
>> +				    ERRINJ_INT);
>> +	if (inj != NULL && inj->iparam > 0)
>> +		mpstream_memset(&stream, 'E', inj->iparam);
> Error injections are supposed to simulate something what can happen
> in reality. Otherwise they might test an impossible case, like here -
> the error object can't be corrupted. If I understood correctly, this

I agree, but I couldn't find a better way to reproduce the issue.

> way you are trying to prove that iproto_write_error() is not called
> at all. Because if it is, you break the data and see it in the test.
> You could make it panic() with the same result I suppose. It is one way.

What I'm trying to do is to achieve the exact same error.

I am trying to write an incomplete row, indeed, but that's not enough to 
trigger
the error: once replica receives an incomplete row, it just exits with
"Unexpected EOF when reading from socket", which is recoverable.

In order to receive ER_INVALID_MSGPACK, not only we need to write an 
incomplete row,
we also need len(incomplete row) + len(iproto error) >= len(original row)

This way length check passes and replication is broken on 
ER_INVALID_MSGPACK,
because obviously you can't have an error object in the middle of other 
row body.

Unfortunately I could only achieve writing an incomplete row with 
relatively large rows
(60 kb in one case and ~ 8 kb in the other). The row is usually split in 
half, so even
while the problem is here (replica receives an iproto error right in the 
middle of another row),
it can't be tested, because all we see is "Unexpected EOF". (8kb / 2 + 
180  < 8kb)

I tried to turn off length checks on the replica with an error 
injection, but stumbled upon a
couple of problems. So I decided to make the error object huge, just so 
it would pass
the length check when written after the partial row.

This way the exact error we need is triggered, ER_INVALID_MSGPACK. And 
it's triggered
not because iproto error contains garbage, but because the row is padded 
with an
iproto error. I checked with verbose logs, and the broken row had the 
beginning of a normal
row, and an iproto error instead of the rest of the row body.
> Another way would be to simulate exactly the happened case - fill the
> buffer with an incomplete xrow.
>
> You tried to do it, from what I see in the test. You create a big
> tuple which should fill the buffer.

>
> But still it didn't help somewhy and you added the invalid error
> object, right?

Yes, as described above.

>
> I managed to remove this injection and still make the test fail 100%
> without your patch. I also think I found an explanation of why the
> corrupted error object was needed before.

>
> See my comment in the test.
>
>> diff --git a/test/replication/gh-4040-invalid-msgpack.result b/test/replication/gh-4040-invalid-msgpack.result
>> new file mode 100644
>> index 000000000..4348ebd90
>> --- /dev/null
>> +++ b/test/replication/gh-4040-invalid-msgpack.result
>> @@ -0,0 +1,180 @@
> <...>
>
>> +-- Generate enough data to fill the sendbuf.
>> +-- This will make the relay yield in the middle of writing a row waiting for the
>> +-- socket to become writeable.
>> +tbl = {1}
>> + | ---
>> + | ...
>> +filler = string.rep('b', 100)
>> + | ---
>> + | ...
>> +for i = 2, 0.7 * bufsize / 100 do\
>> +    tbl[i] = filler\
> The reason why you needed the corrupted error object is that the
> tuples are not big enough.
>
> Consider my machine (I think you should have the same values). I
> have bufsize = 8192 (8KB). Now consider the tuples you create: each
> is < 6KB. Each tuple even with all the MessagePack meta fits into
> the buffer alone.

My buffer size is 8kb as well, yes.

>
> Relay writes the xrows using writev(). On my machine this is what I
> see when I print its results on your tuples:
>
> 	Written 5744
> 	Written 5744
>
> So the xrows are written one by one, not in parts. I don't know why.
> Probably in the kernel it does not accept even a part of the buffer
> if sees it does not fit. So you never had in this test a partially
> written xrow.
That's strange. I did have partially written rows. And I could only 
reproduce the error
reliably only with row size 0.7 * bufsize.

I checked the logs (there's a hex dump of the row in verbose mode),
the error is due to partially written rows, not due to 
IPROTO_WRITE_ERROR_LARGE.


>
> I changed it slightly:
>
> 	-for i = 2, 0.7 * bufsize / 100 do\
> 	+for i = 2, bufsize / 100 + 1 do\
>
> And removed your ERRINJ_IPROTO_WRITE_ERROR_LARGE injection. Now
> each tuple is > 8KB. The kernel is forced to write it in parts.
> This is what I see:
>
> 	Written 8192
> 	Written 102
>
> The total size of one xrow is > 8KB. The kernel allows to write a
> part of the tuple, and now any error, even a valid one, will corrupt
> the buffer. That happens if I revert your timeout error ignorance.
>
> What I don't get is why did you have 0.7 coefficient in the loop?

With other row lengths the reproducer was not stable,
Tarantool didn't fail every time. It failed once in a couple of runs.

And again, I know the large error errinj looks strange, but it doesn't
make the test fail due to the errinj. The test fails due to partially 
written rows.

>
> My full diff:

Cool! I tried your version and it does exactly what's needed. Applied.

I don't know why that didn't work out when I first tried this approach.
I guess you really need row_size ~ buf size,
this way the part of the row that doesn't fit is small (< iproto error 
size),
and normal iproto error on top is enough to trigger the error.

Thanks for the help!

>
> ====================
> diff --git a/src/box/xrow.c b/src/box/xrow.c
> index c4647c6cb..db72437a9 100644
> --- a/src/box/xrow.c
> +++ b/src/box/xrow.c
> @@ -550,10 +550,6 @@ iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
>   
>   	size_t region_svp = region_used(region);
>   	mpstream_iproto_encode_error(&stream, e);
> -	struct errinj *inj = errinj(ERRINJ_IPROTO_WRITE_ERROR_LARGE,
> -				    ERRINJ_INT);
> -	if (inj != NULL && inj->iparam > 0)
> -		mpstream_memset(&stream, 'E', inj->iparam);
>   	mpstream_flush(&stream);
>   	if (is_error)
>   		goto cleanup;
> diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
> index 0d8ec967d..e49242807 100644
> --- a/src/lib/core/errinj.h
> +++ b/src/lib/core/errinj.h
> @@ -153,7 +153,6 @@ struct errinj {
>   	_(ERRINJ_SNAP_COMMIT_FAIL, ERRINJ_BOOL, {.bparam = false}) \
>   	_(ERRINJ_IPROTO_SINGLE_THREAD_STAT, ERRINJ_INT, {.iparam = -1}) \
>   	_(ERRINJ_IPROTO_WRITE_ERROR_DELAY, ERRINJ_BOOL, {.bparam = false})\
> -	_(ERRINJ_IPROTO_WRITE_ERROR_LARGE, ERRINJ_INT, {.iparam = -1})\
>   	_(ERRINJ_APPLIER_READ_TX_ROW_DELAY, ERRINJ_BOOL, {.bparam = false})\
>   
>   ENUM0(errinj_id, ERRINJ_LIST);
> diff --git a/test/box/errinj.result b/test/box/errinj.result
> index e5193a3d3..44f86a54e 100644
> --- a/test/box/errinj.result
> +++ b/test/box/errinj.result
> @@ -61,7 +61,6 @@ evals
>     - ERRINJ_IPROTO_SINGLE_THREAD_STAT: -1
>     - ERRINJ_IPROTO_TX_DELAY: false
>     - ERRINJ_IPROTO_WRITE_ERROR_DELAY: false
> -  - ERRINJ_IPROTO_WRITE_ERROR_LARGE: -1
>     - ERRINJ_LOG_ROTATE: false
>     - ERRINJ_MEMTX_DELAY_GC: false
>     - ERRINJ_PORT_DUMP: false
> diff --git a/test/replication/gh-4040-invalid-msgpack.result b/test/replication/gh-4040-invalid-msgpack.result
> index 4348ebd90..def47ed3c 100644
> --- a/test/replication/gh-4040-invalid-msgpack.result
> +++ b/test/replication/gh-4040-invalid-msgpack.result
> @@ -47,9 +47,6 @@ test_run:switch('replica')
>    | - true
>    | ...
>   
> -box.cfg{log_level=6}
> - | ---
> - | ...
>   sign = box.info.signature
>    | ---
>    | ...
> @@ -81,10 +78,6 @@ box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', true)
>    | ---
>    | - ok
>    | ...
> -box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', bufsize)
> - | ---
> - | - ok
> - | ...
>   -- Generate enough data to fill the sendbuf.
>   -- This will make the relay yield in the middle of writing a row waiting for the
>   -- socket to become writeable.
> @@ -94,7 +87,7 @@ tbl = {1}
>   filler = string.rep('b', 100)
>    | ---
>    | ...
> -for i = 2, 0.7 * bufsize / 100 do\
> +for i = 2, bufsize / 100 + 1 do\
>       tbl[i] = filler\
>   end
>    | ---
> @@ -136,10 +129,6 @@ box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', false)
>    | ---
>    | - ok
>    | ...
> -box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', -1)
> - | ---
> - | - ok
> - | ...
>   
>   test_run:switch('replica')
>    | ---
> diff --git a/test/replication/gh-4040-invalid-msgpack.test.lua b/test/replication/gh-4040-invalid-msgpack.test.lua
> index 0e51affac..f7f1c3953 100644
> --- a/test/replication/gh-4040-invalid-msgpack.test.lua
> +++ b/test/replication/gh-4040-invalid-msgpack.test.lua
> @@ -18,7 +18,6 @@ _ = box.space.test:create_index('pk')
>   test_run:cmd('start server replica with args="1000"')
>   test_run:switch('replica')
>   
> -box.cfg{log_level=6}
>   sign = box.info.signature
>   box.error.injection.set('ERRINJ_APPLIER_READ_TX_ROW_DELAY', true)
>   test_run:switch('master')
> @@ -31,13 +30,12 @@ bufsize = soc:getsockopt('SOL_SOCKET', 'SO_SNDBUF')
>   require('log').info("SO_SNDBUF size is %d", bufsize)
>   -- Master shouldn't try to write the error while the socket isn't writeable.
>   box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', true)
> -box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', bufsize)
>   -- Generate enough data to fill the sendbuf.
>   -- This will make the relay yield in the middle of writing a row waiting for the
>   -- socket to become writeable.
>   tbl = {1}
>   filler = string.rep('b', 100)
> -for i = 2, 0.7 * bufsize / 100 do\
> +for i = 2, bufsize / 100 + 1 do\
>       tbl[i] = filler\
>   end
>   for i = 1,10 do\
> @@ -57,7 +55,6 @@ test_run:wait_cond(function() return box.info.signature > sign end)
>   
>   test_run:switch('master')
>   box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_DELAY', false)
> -box.error.injection.set('ERRINJ_IPROTO_WRITE_ERROR_LARGE', -1)
>   
>   test_run:switch('replica')
>   

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] replication: stop pushing TimedOut error to the replica

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the fixes!

LGTM.