From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Ilya Markov Subject: [PATCH 2/2 v4] xlog: Remove inprogress files on start Date: Wed, 20 Jun 2018 16:48:03 +0300 Message-Id: In-Reply-To: References: <20180620120853.4nell7bxgbmjjffy@esperanza> In-Reply-To: References: To: vdavydov.dev@gmail.com Cc: georgy@tarantool.org, tarantool-patches@freelists.org List-ID: When tarantool crashes during writing to vylog, index, run or snapshot, inprogress files remain. But garbage collector doesn't take into account these files. So they remain until they are removed manually. Fix this with adding deletion of such files to functions memtx_engine_end_recovery, vinyl_engine_end_recovery and vy_run_remove_files. - Add 3 errinj which simulate the crash before renaming inprogress files. - Moved const string inprogress_suffix to xlog.h and replace it with #defiled string in order to reuse it in vinyl run and index files removal. - Add 2 new vy_file_type in order to support formatting of inprogress files names. Closes #3406 --- src/box/memtx_engine.c | 1 + src/box/vinyl.c | 3 + src/box/vy_log.c | 7 +- src/box/vy_run.c | 19 +++++- src/box/vy_run.h | 2 + src/box/xlog.c | 29 +++++++- src/box/xlog.h | 11 +++ src/errinj.h | 3 + test/box/errinj.result | 171 +++++++++++++++++++++++++++++++++++++++++++++++ test/box/errinj.test.lua | 64 ++++++++++++++++++ 10 files changed, 305 insertions(+), 5 deletions(-) diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c index 9a9aff5..d211bcd 100644 --- a/src/box/memtx_engine.c +++ b/src/box/memtx_engine.c @@ -303,6 +303,7 @@ memtx_engine_end_recovery(struct engine *engine) if (space_foreach(memtx_build_secondary_keys, memtx) != 0) return -1; } + xdir_collect_inprogress(&memtx->snap_dir); return 0; } diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 552d42b..c82b7cd 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -3481,6 +3481,9 @@ vy_backup_cb(const struct vy_log_record *record, void *cb_arg) char path[PATH_MAX]; for (int type = 0; type < vy_file_MAX; type++) { + if (type == VY_FILE_RUN_INPROGRESS || + type == VY_FILE_INDEX_INPROGRESS) + continue; vy_run_snprint_path(path, sizeof(path), arg->env->path, arg->space_id, arg->index_id, record->run_id, type); diff --git a/src/box/vy_log.c b/src/box/vy_log.c index 51e3753..7a7993d 100644 --- a/src/box/vy_log.c +++ b/src/box/vy_log.c @@ -852,6 +852,10 @@ vy_log_open(struct xlog *xlog) xlog_write_row(xlog, &row) < 0) goto fail_close_xlog; + ERROR_INJECT(ERRINJ_VY_LOG_FILE_RENAME, { + diag_set(ClientError, ER_INJECTION, "vinyl log file rename"); + goto fail_errinj; + }); if (xlog_rename(xlog) < 0) goto fail_close_xlog; @@ -860,6 +864,7 @@ vy_log_open(struct xlog *xlog) fail_close_xlog: if (unlink(xlog->filename) < 0) say_syserror("failed to delete file '%s'", xlog->filename); +fail_errinj: xlog_close(xlog, false); fail: return -1; @@ -976,7 +981,7 @@ vy_log_end_recovery(void) return -1; } } - + xdir_collect_inprogress(&vy_log.dir); vy_log.recovery = NULL; return 0; } diff --git a/src/box/vy_run.c b/src/box/vy_run.c index 2c5c0c9..0626e28 100644 --- a/src/box/vy_run.c +++ b/src/box/vy_run.c @@ -66,8 +66,10 @@ enum { VY_BLOOM_VERSION = 0 }; #define XLOG_META_TYPE_INDEX "INDEX" const char *vy_file_suffix[] = { - "index", /* VY_FILE_INDEX */ - "run", /* VY_FILE_RUN */ + "index", /* VY_FILE_INDEX */ + "index" inprogress_suffix, /* VY_FILE_INDEX_INPROGRESS */ + "run", /* VY_FILE_RUN */ + "run" inprogress_suffix, /* VY_FILE_RUN_INPROGRESS */ }; /** @@ -2021,6 +2023,11 @@ vy_run_write_index(struct vy_run *run, const char *dirpath, goto fail; } + ERROR_INJECT(ERRINJ_VY_INDEX_FILE_RENAME, { + diag_set(ClientError, ER_INJECTION, + "vinyl index file rename"); + goto fail_errinj; + }); if (xlog_tx_commit(&index_xlog) < 0 || xlog_flush(&index_xlog) < 0 || xlog_rename(&index_xlog) < 0) @@ -2029,10 +2036,11 @@ vy_run_write_index(struct vy_run *run, const char *dirpath, region_truncate(region, mem_used); return 0; fail: + unlink(path); +fail_errinj: region_truncate(region, mem_used); xlog_tx_rollback(&index_xlog); xlog_close(&index_xlog, false); - unlink(path); return -1; } @@ -2263,6 +2271,11 @@ vy_run_writer_commit(struct vy_run_writer *writer) run->info.max_key = vy_key_dup(key); if (run->info.max_key == NULL) goto out; + ERROR_INJECT(ERRINJ_VY_RUN_FILE_RENAME, { + diag_set(ClientError, ER_INJECTION, + "vinyl run file rename"); + goto out; + }); /* Sync data and link the file to the final name. */ if (xlog_sync(&writer->data_xlog) < 0 || diff --git a/src/box/vy_run.h b/src/box/vy_run.h index b3e3d1c..6afef2a 100644 --- a/src/box/vy_run.h +++ b/src/box/vy_run.h @@ -378,7 +378,9 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir, enum vy_file_type { VY_FILE_INDEX, + VY_FILE_INDEX_INPROGRESS, VY_FILE_RUN, + VY_FILE_RUN_INPROGRESS, vy_file_MAX, }; diff --git a/src/box/xlog.c b/src/box/xlog.c index 04f8587..142cdcf 100644 --- a/src/box/xlog.c +++ b/src/box/xlog.c @@ -58,7 +58,6 @@ typedef uint32_t log_magic_t; static const log_magic_t row_marker = mp_bswap_u32(0xd5ba0bab); /* host byte order */ static const log_magic_t zrow_marker = mp_bswap_u32(0xd5ba0bba); /* host byte order */ static const log_magic_t eof_marker = mp_bswap_u32(0xd510aded); /* host byte order */ -static const char inprogress_suffix[] = ".inprogress"; enum { /** @@ -625,6 +624,34 @@ xdir_collect_garbage(struct xdir *dir, int64_t signature, bool use_coio) return 0; } +void +xdir_collect_inprogress(struct xdir *xdir) +{ + const char *dirname = xdir->dirname; + DIR *dh = opendir(dirname); + if (dh == NULL) { + if (errno != ENOENT) + say_syserror("error reading directory '%s'", dirname); + return; + } + + struct dirent *dent; + char path[PATH_MAX]; + int total = snprintf(path, sizeof(path), "%s/", dirname); + + while ((dent = readdir(dh)) != NULL) { + char *ext = strrchr(dent->d_name, '.'); + if (ext == NULL || strcmp(ext, inprogress_suffix) != 0) + continue; + strncpy(path + total, dent->d_name, sizeof(path) - total); + if (unlink(path) < 0) { + say_syserror("error while removing %s", path); + return; + } else + say_info("removed %s", path); + } +} + /* }}} */ diff --git a/src/box/xlog.h b/src/box/xlog.h index 973910d..3b03588 100644 --- a/src/box/xlog.h +++ b/src/box/xlog.h @@ -70,6 +70,11 @@ enum xdir_type { enum log_suffix { NONE, INPROGRESS }; /** + * Suffix added to path of inprogress files. + */ +#define inprogress_suffix ".inprogress" + +/** * A handle for a data directory with write ahead logs, snapshots, * vylogs. * Can be used to find the last log in the directory, scan @@ -181,6 +186,12 @@ int xdir_collect_garbage(struct xdir *dir, int64_t signature, bool use_coio); /** + * Remove .inprogress files in specified directory. + */ +void +xdir_collect_inprogress(struct xdir *xdir); + +/** * Return LSN and vclock (unless @vclock is NULL) of the newest * file in a directory or -1 if the directory is empty. */ diff --git a/src/errinj.h b/src/errinj.h index 78514ac..7141af9 100644 --- a/src/errinj.h +++ b/src/errinj.h @@ -111,6 +111,9 @@ struct errinj { _(ERRINJ_IPROTO_TX_DELAY, ERRINJ_BOOL, {.bparam = false}) \ _(ERRINJ_LOG_ROTATE, ERRINJ_BOOL, {.bparam = false}) \ _(ERRINJ_SNAP_COMMIT_DELAY, ERRINJ_BOOL, {.bparam = 0}) \ + _(ERRINJ_VY_LOG_FILE_RENAME, ERRINJ_BOOL, {.bparam = false}) \ + _(ERRINJ_VY_INDEX_FILE_RENAME, ERRINJ_BOOL, {.bparam = false}) \ + _(ERRINJ_VY_RUN_FILE_RENAME, 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 6ced172..34a5c69 100644 --- a/test/box/errinj.result +++ b/test/box/errinj.result @@ -4,6 +4,12 @@ errinj = box.error.injection net_box = require('net.box') --- ... +fio = require("fio") +--- +... +fiber = require('fiber') +--- +... space = box.schema.space.create('tweedledum') --- ... @@ -40,8 +46,12 @@ errinj.info() state: false ERRINJ_WAL_IO: state: false + ERRINJ_VY_INDEX_FILE_RENAME: + state: false ERRINJ_TUPLE_ALLOC: state: false + ERRINJ_VY_RUN_FILE_RENAME: + state: false ERRINJ_VY_READ_PAGE: state: false ERRINJ_RELAY_REPORT_INTERVAL: @@ -56,6 +66,8 @@ errinj.info() state: false ERRINJ_VY_LOG_FLUSH_DELAY: state: false + ERRINJ_VY_LOG_FILE_RENAME: + state: false ERRINJ_SNAP_COMMIT_DELAY: state: false ERRINJ_RELAY_FINAL_SLEEP: @@ -693,6 +705,165 @@ errinj.set("ERRINJ_WAL_WRITE", false) space:drop() --- ... +errinj.set("ERRINJ_VY_LOG_FILE_RENAME", true) +--- +- ok +... +s = box.schema.space.create("vinyl_test", {engine='vinyl'}) +--- +... +_ = s:create_index("primary") +--- +... +vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir) +--- +... +#fio.glob(vinyl_vylog) > 0 -- true +--- +- true +... +errinj.set("ERRINJ_SNAP_COMMIT_DELAY", true) +--- +- ok +... +_ = fiber.create(function() box.snapshot() end) +--- +... +memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir) +--- +... +while #fio.glob(memtx_snap) == 0 do fiber.sleep(0.001) end-- true +--- +... +#fio.glob(memtx_snap) > 0 --true +--- +- true +... +test_run:cmd("restart server default") +errinj = box.error.injection +--- +... +net_box = require('net.box') +--- +... +fio = require("fio") +--- +... +fiber = require('fiber') +--- +... +memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir) +--- +... +#fio.glob(memtx_snap) == 0 -- true +--- +- true +... +vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir) +--- +... +#fio.glob(vinyl_vylog) == 0 --true +--- +- true +... +s = box.space.vinyl_test +--- +... +vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id) +--- +... +errinj.set("ERRINJ_VY_INDEX_FILE_RENAME", true) +--- +- ok +... +for i = 1, 10 do s:insert{i} end +--- +... +box.snapshot() +--- +- error: Error injection 'vinyl index file rename' +... +while #fio.glob(vinyl_index) == 0 do fiber.sleep(0.001) end +--- +... +vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id) +--- +... +errinj.set("ERRINJ_VY_RUN_FILE_RENAME", true) +--- +- ok +... +while #fio.glob(vinyl_run) == 0 do fiber.sleep(0.001) end +--- +... +#fio.glob(vinyl_run) > 0 -- true +--- +- true +... +#fio.glob(vinyl_index) > 0 -- true +--- +- true +... +test_run:cmd("restart server default") +errinj = box.error.injection +--- +... +net_box = require('net.box') +--- +... +fio = require("fio") +--- +... +fiber = require('fiber') +--- +... +s = box.space.vinyl_test +--- +... +memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir) +--- +... +vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir) +--- +... +vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id) +--- +... +vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id) +--- +... +-- no inprogress files should be present in memtx(vinyl)_dir. +#fio.glob(memtx_snap) == 0 -- true +--- +- true +... +#fio.glob(vinyl_vylog) == 0 -- true +--- +- true +... +#fio.glob(vinyl_index) == 0 -- true +--- +- true +... +#fio.glob(vinyl_run) == 0 -- true +--- +- true +... +box.space.vinyl_test:drop() +--- +... +run_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/*/*") +--- +... +for _, file in ipairs(fio.glob(run_dir)) do fio.unlink(file) end +--- +... +vinyl_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/") +--- +... +for _, dir in ipairs(fio.glob(vinyl_dir)) do fio.rmtree(dir) end +--- +... --test space:bsize() in case of memory error utils = dofile('utils.lua') --- diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua index 3af1b74..647fb0a 100644 --- a/test/box/errinj.test.lua +++ b/test/box/errinj.test.lua @@ -1,5 +1,7 @@ errinj = box.error.injection net_box = require('net.box') +fio = require("fio") +fiber = require('fiber') space = box.schema.space.create('tweedledum') index = space:create_index('primary', { type = 'hash' }) @@ -203,6 +205,68 @@ box.snapshot() errinj.set("ERRINJ_WAL_WRITE", false) space:drop() +errinj.set("ERRINJ_VY_LOG_FILE_RENAME", true) +s = box.schema.space.create("vinyl_test", {engine='vinyl'}) +_ = s:create_index("primary") +vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir) +#fio.glob(vinyl_vylog) > 0 -- true + +errinj.set("ERRINJ_SNAP_COMMIT_DELAY", true) +_ = fiber.create(function() box.snapshot() end) +memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir) +while #fio.glob(memtx_snap) == 0 do fiber.sleep(0.001) end-- true +#fio.glob(memtx_snap) > 0 --true + +test_run:cmd("restart server default") +errinj = box.error.injection +net_box = require('net.box') +fio = require("fio") +fiber = require('fiber') + +memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir) +#fio.glob(memtx_snap) == 0 -- true + +vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir) +#fio.glob(vinyl_vylog) == 0 --true + +s = box.space.vinyl_test +vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id) +errinj.set("ERRINJ_VY_INDEX_FILE_RENAME", true) + +for i = 1, 10 do s:insert{i} end +box.snapshot() + +while #fio.glob(vinyl_index) == 0 do fiber.sleep(0.001) end + +vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id) +errinj.set("ERRINJ_VY_RUN_FILE_RENAME", true) +while #fio.glob(vinyl_run) == 0 do fiber.sleep(0.001) end +#fio.glob(vinyl_run) > 0 -- true +#fio.glob(vinyl_index) > 0 -- true + +test_run:cmd("restart server default") +errinj = box.error.injection +net_box = require('net.box') +fio = require("fio") +fiber = require('fiber') + +s = box.space.vinyl_test +memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir) +vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir) +vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id) +vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id) + +-- no inprogress files should be present in memtx(vinyl)_dir. +#fio.glob(memtx_snap) == 0 -- true +#fio.glob(vinyl_vylog) == 0 -- true +#fio.glob(vinyl_index) == 0 -- true +#fio.glob(vinyl_run) == 0 -- true + +box.space.vinyl_test:drop() +run_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/*/*") +for _, file in ipairs(fio.glob(run_dir)) do fio.unlink(file) end +vinyl_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/") +for _, dir in ipairs(fio.glob(vinyl_dir)) do fio.rmtree(dir) end --test space:bsize() in case of memory error utils = dofile('utils.lua') s = box.schema.space.create('space_bsize') -- 2.7.4