From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 3/9] recovery: restore garbage collector vclock after restart Date: Wed, 28 Nov 2018 19:14:41 +0300 Message-Id: <67678dc51f6a47c48dfb2d2a52a2b059514bd767.1543419109.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: After restart the garbage collector vclock is reset to the vclock of the oldest preserved checkpoint, which is incorrect - it may be less in case there is a replica that lagged behind, and it may be greater as well in case the WAL thread hit ENOSPC and had to remove some WAL files to continue. Fix it. A note about xlog/panic_on_wal_error test. To check that replication stops if some xlogs are missing, the test first removes xlogs on the master, then restarts the master, then tries to start the replica expecting that replication should fail. Well, it shouldn't - the replica should rebootstrap instead. It didn't rebootstrap before this patch though, because the master reported wrong garbage collector vclock (as it didn't recover it on restart). After this patch the replica would rebootstrap and the test would hang. Fix this by restarting the master before removing xlog files. --- src/box/box.cc | 2 +- src/box/recovery.cc | 14 +++++++++----- src/box/recovery.h | 5 ++++- test/replication/gc_no_space.result | 15 +++++++++++---- test/replication/gc_no_space.test.lua | 7 +++++-- test/xlog/panic_on_wal_error.result | 2 +- test/xlog/panic_on_wal_error.test.lua | 2 +- 7 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/box/box.cc b/src/box/box.cc index 72788f82..a5e1f07a 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -1918,7 +1918,7 @@ local_recovery(const struct tt_uuid *instance_uuid, * so we must reflect this in replicaset vclock to * not attempt to apply these rows twice. */ - recovery_scan(recovery, &replicaset.vclock); + recovery_scan(recovery, &replicaset.vclock, &gc.vclock); if (wal_dir_lock >= 0) { box_listen(); diff --git a/src/box/recovery.cc b/src/box/recovery.cc index 77357f04..64d50989 100644 --- a/src/box/recovery.cc +++ b/src/box/recovery.cc @@ -117,21 +117,25 @@ recovery_new(const char *wal_dirname, bool force_recovery, } void -recovery_scan(struct recovery *r, struct vclock *end_vclock) +recovery_scan(struct recovery *r, struct vclock *end_vclock, + struct vclock *gc_vclock) { xdir_scan_xc(&r->wal_dir); - struct vclock *vclock = vclockset_last(&r->wal_dir.index); - if (vclock == NULL || vclock_compare(vclock, &r->vclock) < 0) { + if (xdir_last_vclock(&r->wal_dir, end_vclock) < 0 || + vclock_compare(end_vclock, &r->vclock) < 0) { /* No xlogs after last checkpoint. */ + vclock_copy(gc_vclock, &r->vclock); vclock_copy(end_vclock, &r->vclock); return; } + if (xdir_first_vclock(&r->wal_dir, gc_vclock) < 0) + unreachable(); + /* Scan the last xlog to find end vclock. */ - vclock_copy(end_vclock, vclock); struct xlog_cursor cursor; - if (xdir_open_cursor(&r->wal_dir, vclock_sum(vclock), &cursor) != 0) + if (xdir_open_cursor(&r->wal_dir, vclock_sum(end_vclock), &cursor) != 0) return; struct xrow_header row; while (xlog_cursor_next(&cursor, &row, true) == 0) diff --git a/src/box/recovery.h b/src/box/recovery.h index 5882d969..662be3ca 100644 --- a/src/box/recovery.h +++ b/src/box/recovery.h @@ -72,9 +72,12 @@ recovery_delete(struct recovery *r); * Scan the WAL directory, build an index of all found * WAL files, then scan the most recent WAL file to find * the vclock of the last record (returned in @end_vclock). + * @gc_vclock is set to the oldest vclock available in the + * WAL directory. */ void -recovery_scan(struct recovery *r, struct vclock *end_vclock); +recovery_scan(struct recovery *r, struct vclock *end_vclock, + struct vclock *gc_vclock); void recovery_follow_local(struct recovery *r, struct xstream *stream, diff --git a/test/replication/gc_no_space.result b/test/replication/gc_no_space.result index 5c64bea4..b2d3e207 100644 --- a/test/replication/gc_no_space.result +++ b/test/replication/gc_no_space.result @@ -43,9 +43,6 @@ test_run:cmd("setopt delimiter ''"); --- - true ... -default_checkpoint_count = box.cfg.checkpoint_count ---- -... box.cfg{checkpoint_count = 2} --- ... @@ -264,6 +261,16 @@ box.schema.user.revoke('guest', 'replication') test_run:cleanup_cluster() --- ... -box.cfg{checkpoint_count = default_checkpoint_count} +-- Check that the garbage collector vclock is recovered correctly. +test_run:cmd("restart server default") +gc = box.info.gc() +--- +... +#gc.checkpoints -- 2 --- +- 2 +... +gc.signature == gc.checkpoints[2].signature +--- +- true ... diff --git a/test/replication/gc_no_space.test.lua b/test/replication/gc_no_space.test.lua index 7f5ab803..6940996f 100644 --- a/test/replication/gc_no_space.test.lua +++ b/test/replication/gc_no_space.test.lua @@ -26,7 +26,6 @@ function check_snap_count(count) end; test_run:cmd("setopt delimiter ''"); -default_checkpoint_count = box.cfg.checkpoint_count box.cfg{checkpoint_count = 2} test_run:cleanup_cluster() @@ -111,4 +110,8 @@ s:drop() box.schema.user.revoke('guest', 'replication') test_run:cleanup_cluster() -box.cfg{checkpoint_count = default_checkpoint_count} +-- Check that the garbage collector vclock is recovered correctly. +test_run:cmd("restart server default") +gc = box.info.gc() +#gc.checkpoints -- 2 +gc.signature == gc.checkpoints[2].signature diff --git a/test/xlog/panic_on_wal_error.result b/test/xlog/panic_on_wal_error.result index 345534ba..22f14f91 100644 --- a/test/xlog/panic_on_wal_error.result +++ b/test/xlog/panic_on_wal_error.result @@ -67,6 +67,7 @@ test_run:cmd("stop server replica") --- - true ... +test_run:cmd("restart server default") box.space.test:auto_increment{'after snapshot'} --- - [2, 'after snapshot'] @@ -94,7 +95,6 @@ files = fio.glob(glob) for _, file in pairs(files) do fio.unlink(file) end --- ... -test_run:cmd("restart server default") -- -- make sure the server has some xlogs, otherwise the -- replica doesn't discover the gap in the logs diff --git a/test/xlog/panic_on_wal_error.test.lua b/test/xlog/panic_on_wal_error.test.lua index 29410cb2..2e95431c 100644 --- a/test/xlog/panic_on_wal_error.test.lua +++ b/test/xlog/panic_on_wal_error.test.lua @@ -31,6 +31,7 @@ box.space.test:select{} -- test_run:cmd("switch default") test_run:cmd("stop server replica") +test_run:cmd("restart server default") box.space.test:auto_increment{'after snapshot'} box.space.test:auto_increment{'after snapshot - one more row'} -- @@ -41,7 +42,6 @@ fio = require('fio') glob = fio.pathjoin(box.cfg.wal_dir, '*.xlog') files = fio.glob(glob) for _, file in pairs(files) do fio.unlink(file) end -test_run:cmd("restart server default") -- -- make sure the server has some xlogs, otherwise the -- replica doesn't discover the gap in the logs -- 2.11.0