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 13/13] replication: ref checkpoint needed to join replica
Date: Thu,  4 Oct 2018 20:20:15 +0300	[thread overview]
Message-ID: <bc67af6519aef89779a9859848ef4795ef787083.1538671546.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1538671546.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1538671546.git.vdavydov.dev@gmail.com>

Before joining a new replica we register a gc_consumer to prevent
garbage collection of files needed for join and following subscribe.
Before commit 9c5d851d7830 ("replication: remove old snapshot files not
needed by replicas") a consumer would pin both checkpoints and WALs so
that would work as expected. However, the above mentioned commit
introduced consumer types and marked a consumer registered on replica
join as WAL-only so if the garbage collector was invoked during join, it
could delete files corresponding to the relayed checkpoint resulting in
replica join failure. Fix this issue by pinning the checkpoint used for
joining a replica with gc_ref_checkpoint and unpinning once join is
complete.

The issue can only be reproduced if there are vinyl spaces, because
deletion of an open snap file doesn't prevent the relay from reading it.
The existing replication/gc test would catch the issue if it triggered
compaction on the master so we simply tweak it accordingly instead of
adding a new test case.

Closes #3708
---
 src/box/box.cc               | 26 ++++++++++++++++----------
 test/replication/gc.result   | 19 +++++++++++++------
 test/replication/gc.test.lua |  8 +++++---
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 2679f6da..7e32b9fc 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1465,14 +1465,14 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
 	struct vclock start_vclock;
 	vclock_copy(&start_vclock, &checkpoint->vclock);
 
-	/* Register the replica with the garbage collector. */
-	struct gc_consumer *gc = gc_consumer_register(&start_vclock,
-			"replica %s", tt_uuid_str(&instance_uuid));
-	if (gc == NULL)
-		diag_raise();
-	auto gc_guard = make_scoped_guard([=]{
-		gc_consumer_unregister(gc);
-	});
+	/*
+	 * Make sure the checkpoint files won't be deleted while
+	 * initial join is in progress.
+	 */
+	struct gc_checkpoint_ref gc;
+	gc_ref_checkpoint(checkpoint, &gc, "replica %s",
+			  tt_uuid_str(&instance_uuid));
+	auto gc_guard = make_scoped_guard([&]{ gc_unref_checkpoint(&gc); });
 
 	/* Respond to JOIN request with start_vclock. */
 	struct xrow_header row;
@@ -1496,8 +1496,14 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
 
 	replica = replica_by_uuid(&instance_uuid);
 	assert(replica != NULL);
-	replica->gc = gc;
-	gc_guard.is_active = false;
+
+	/* Register the replica as a WAL consumer. */
+	if (replica->gc != NULL)
+		gc_consumer_unregister(replica->gc);
+	replica->gc = gc_consumer_register(&start_vclock, "replica %s",
+					   tt_uuid_str(&instance_uuid));
+	if (replica->gc == NULL)
+		diag_raise();
 
 	/* Remember master's vclock after the last request */
 	struct vclock stop_vclock;
diff --git a/test/replication/gc.result b/test/replication/gc.result
index 83d0de29..5944ba51 100644
--- a/test/replication/gc.result
+++ b/test/replication/gc.result
@@ -16,6 +16,10 @@ fiber = require('fiber')
 test_run:cleanup_cluster()
 ---
 ...
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+---
+- true
+...
 -- Make each snapshot trigger garbage collection.
 default_checkpoint_count = box.cfg.checkpoint_count
 ---
@@ -40,10 +44,17 @@ box.error.injection.set("ERRINJ_RELAY_REPORT_INTERVAL", 0.05)
 s = box.schema.space.create('test', {engine = engine});
 ---
 ...
-_ = s:create_index('pk')
+_ = s:create_index('pk', {run_count_per_level = 1})
 ---
 ...
-for i = 1, 100 do s:auto_increment{} end
+for i = 1, 50 do s:auto_increment{} end
+---
+...
+box.snapshot()
+---
+- ok
+...
+for i = 1, 50 do s:auto_increment{} end
 ---
 ...
 box.snapshot()
@@ -76,10 +87,6 @@ test_run:cmd("setopt delimiter ''");
 ---
 ...
 -- Start the replica.
-test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
----
-- true
-...
 test_run:cmd("start server replica")
 ---
 - true
diff --git a/test/replication/gc.test.lua b/test/replication/gc.test.lua
index eed76850..84c980dd 100644
--- a/test/replication/gc.test.lua
+++ b/test/replication/gc.test.lua
@@ -5,6 +5,7 @@ replica_set = require('fast_replica')
 fiber = require('fiber')
 
 test_run:cleanup_cluster()
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
 
 -- Make each snapshot trigger garbage collection.
 default_checkpoint_count = box.cfg.checkpoint_count
@@ -21,8 +22,10 @@ box.error.injection.set("ERRINJ_RELAY_REPORT_INTERVAL", 0.05)
 
 -- Create and populate the space we will replicate.
 s = box.schema.space.create('test', {engine = engine});
-_ = s:create_index('pk')
-for i = 1, 100 do s:auto_increment{} end
+_ = s:create_index('pk', {run_count_per_level = 1})
+for i = 1, 50 do s:auto_increment{} end
+box.snapshot()
+for i = 1, 50 do s:auto_increment{} end
 box.snapshot()
 for i = 1, 100 do s:auto_increment{} end
 
@@ -43,7 +46,6 @@ end)
 test_run:cmd("setopt delimiter ''");
 
 -- Start the replica.
-test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
 test_run:cmd("start server replica")
 
 -- Despite the fact that we invoked garbage collection that
-- 
2.11.0

  parent reply	other threads:[~2018-10-04 17:20 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-04 17:20 [PATCH 00/13] box: garbage collection refactoring and fixes Vladimir Davydov
2018-10-04 17:20 ` [PATCH 01/13] vinyl: fix master crash on replica join failure Vladimir Davydov
2018-10-04 21:43   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 02/13] vinyl: force deletion of runs left from unfinished indexes on restart Vladimir Davydov
2018-10-04 21:44   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 03/13] gc: make gc_consumer and gc_state structs transparent Vladimir Davydov
2018-10-04 21:47   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 04/13] gc: use fixed length buffer for storing consumer name Vladimir Davydov
2018-10-04 21:47   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 05/13] gc: fold gc_consumer_new and gc_consumer_delete Vladimir Davydov
2018-10-04 21:50   ` Konstantin Osipov
2018-10-05  8:56     ` Vladimir Davydov
2018-10-04 17:20 ` [PATCH 06/13] gc: format consumer name in gc_consumer_register Vladimir Davydov
2018-10-04 21:50   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 07/13] gc: rename checkpoint_count to min_checkpoint_count Vladimir Davydov
2018-10-04 21:51   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 08/13] gc: keep track of available checkpoints Vladimir Davydov
2018-10-04 21:59   ` Konstantin Osipov
2018-10-05  8:50     ` Vladimir Davydov
2018-10-04 17:20 ` [PATCH 09/13] gc: cleanup garbage collection procedure Vladimir Davydov
2018-10-04 22:00   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 10/13] gc: improve box.info.gc output Vladimir Davydov
2018-10-04 22:01   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 11/13] gc: separate checkpoint references from wal consumers Vladimir Davydov
2018-10-04 22:05   ` Konstantin Osipov
2018-10-04 17:20 ` [PATCH 12/13] gc: call gc_run unconditionally when consumer is advanced Vladimir Davydov
2018-10-04 22:26   ` Konstantin Osipov
2018-10-04 17:20 ` Vladimir Davydov [this message]
2018-10-04 22:27   ` [PATCH 13/13] replication: ref checkpoint needed to join replica Konstantin Osipov
2018-10-05 17:03 ` [PATCH 00/13] box: garbage collection refactoring and fixes 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=bc67af6519aef89779a9859848ef4795ef787083.1538671546.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 13/13] replication: ref checkpoint needed to join replica' \
    /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