Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: kostja@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [PATCH 4/6] box: use replicaset.vclock in replica join/subscribe
Date: Sun, 25 Nov 2018 16:48:11 +0300	[thread overview]
Message-ID: <2ab0d1a4bf8a5050ad4ea764b5cab41fd1d5968d.1543152574.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1543152574.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1543152574.git.vdavydov.dev@gmail.com>

Again, this is something that was introduced by commit f2bccc18485d
("Use WAL vclock instead of TX vclock in most places") without any
justification.

TX has its own copy of the current vclock - there's absolutely no need
to inquire it from the WAL thread. Actually, we already use TX local
vclock in box_process_vote(). No reason to treat join/subscribe any
different. Moreover, it's even harmful - there may be a gap at the end
of a WAL file, in which case WAL vclock will be slightly ahead of TX
vclock so that should a replica try to subscribe it would never finish
syncing, see #3830.

Closes #3830
---
 src/box/box.cc                 | 10 +++-------
 test/replication/sync.result   | 37 +++++++++++++++++++++++++++++++++++++
 test/replication/sync.test.lua | 12 ++++++++++++
 3 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 8a1a2668..8d6e966e 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1513,7 +1513,7 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
 
 	/* Remember master's vclock after the last request */
 	struct vclock stop_vclock;
-	wal_checkpoint(&stop_vclock, false);
+	vclock_copy(&stop_vclock, &replicaset.vclock);
 
 	/*
 	 * Register the replica as a WAL consumer so that
@@ -1540,9 +1540,7 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
 	say_info("final data sent.");
 
 	/* Send end of WAL stream marker */
-	struct vclock current_vclock;
-	wal_checkpoint(&current_vclock, false);
-	xrow_encode_vclock_xc(&row, &current_vclock);
+	xrow_encode_vclock_xc(&row, &replicaset.vclock);
 	row.sync = header->sync;
 	coio_write_xrow(io, &row);
 }
@@ -1608,9 +1606,7 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	 * and identify ourselves with our own replica id.
 	 */
 	struct xrow_header row;
-	struct vclock current_vclock;
-	wal_checkpoint(&current_vclock, false);
-	xrow_encode_vclock_xc(&row, &current_vclock);
+	xrow_encode_vclock_xc(&row, &replicaset.vclock);
 	/*
 	 * Identify the message with the replica id of this
 	 * instance, this is the only way for a replica to find
diff --git a/test/replication/sync.result b/test/replication/sync.result
index 4c0ad9c3..dc3a6f69 100644
--- a/test/replication/sync.result
+++ b/test/replication/sync.result
@@ -311,6 +311,43 @@ test_run:cmd("stop server replica")
 ---
 - true
 ...
+-- gh-3830: Sync fails if there's a gap at the end of the master's WAL.
+box.error.injection.set('ERRINJ_WAL_WRITE_DISK', true)
+---
+- ok
+...
+box.space.test:replace{123456789}
+---
+- error: Failed to write to disk
+...
+box.error.injection.set('ERRINJ_WAL_WRITE_DISK', false)
+---
+- ok
+...
+test_run:cmd("start server replica")
+---
+- true
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+box.info.status -- running
+---
+- running
+...
+box.info.ro -- false
+---
+- false
+...
+test_run:cmd("switch default")
+---
+- true
+...
+test_run:cmd("stop server replica")
+---
+- true
+...
 test_run:cmd("cleanup server replica")
 ---
 - true
diff --git a/test/replication/sync.test.lua b/test/replication/sync.test.lua
index ee82fc58..bc714735 100644
--- a/test/replication/sync.test.lua
+++ b/test/replication/sync.test.lua
@@ -158,6 +158,18 @@ test_run:grep_log('replica', 'ER_CFG.*')
 
 test_run:cmd("switch default")
 test_run:cmd("stop server replica")
+
+-- gh-3830: Sync fails if there's a gap at the end of the master's WAL.
+box.error.injection.set('ERRINJ_WAL_WRITE_DISK', true)
+box.space.test:replace{123456789}
+box.error.injection.set('ERRINJ_WAL_WRITE_DISK', false)
+test_run:cmd("start server replica")
+test_run:cmd("switch replica")
+box.info.status -- running
+box.info.ro -- false
+
+test_run:cmd("switch default")
+test_run:cmd("stop server replica")
 test_run:cmd("cleanup server replica")
 
 box.space.test:drop()
-- 
2.11.0

  parent reply	other threads:[~2018-11-25 13:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-25 13:48 [PATCH 0/6] WAL garbage collection and checkpointing fixes Vladimir Davydov
2018-11-25 13:48 ` [PATCH 1/6] vclock: allow to use const vclock as search key Vladimir Davydov
2018-11-26 17:38   ` Konstantin Osipov
2018-11-27  9:56     ` Vladimir Davydov
2018-11-25 13:48 ` [PATCH 2/6] engine: pass vclock instead of lsn to collect_garbage callback Vladimir Davydov
2018-11-26 17:41   ` Konstantin Osipov
2018-11-27  9:56     ` Vladimir Davydov
2018-11-25 13:48 ` [PATCH 3/6] box: do not rotate WAL when replica subscribes Vladimir Davydov
2018-11-26 17:50   ` Konstantin Osipov
2018-11-27  9:57     ` Vladimir Davydov
2018-11-25 13:48 ` Vladimir Davydov [this message]
2018-11-26 17:54   ` [PATCH 4/6] box: use replicaset.vclock in replica join/subscribe Konstantin Osipov
2018-11-27  9:57     ` Vladimir Davydov
2018-11-25 13:48 ` [PATCH 5/6] wal: separate checkpoint and flush paths Vladimir Davydov
2018-11-26 17:58   ` Konstantin Osipov
2018-11-26 20:19     ` Vladimir Davydov
2018-11-28 16:46       ` Konstantin Osipov
2018-11-25 13:48 ` [PATCH 6/6] wal: remove files needed for recovery from backup checkpoints on ENOSPC Vladimir Davydov
2018-11-28 16:14   ` 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=2ab0d1a4bf8a5050ad4ea764b5cab41fd1d5968d.1543152574.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 4/6] box: use replicaset.vclock in replica join/subscribe' \
    /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