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
next prev 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