<HTML><BODY><div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<base target="_self" href="https://e.mail.ru/">
<div id="style_15799887170149448598_BODY"><div class="class_1580791522">
<div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix">
<style></style>
<div>
<div id="style_15799593960768587720_BODY_mailru_css_attribute_postfix"><div class="class_1579961688_mailru_css_attribute_postfix">
Hi!<br><br>> Is there any issue corresponding to the problem you are solving?<br><br>No, there is no issue corresponding to that problem.<br><br>> > ---<br>> > src/box/vy_lsm.c | 19 +-<br>> > test/vinyl/delete_run.result | 302 +++++++++++++++++++++++++++++<br>> > test/vinyl/delete_run.test.lua | 93 +++++++++<br>> > test/vinyl/force_recovery_true.lua | 11 ++<br>> > 4 files changed, 421 insertions(+), 4 deletions(-)<br>> > create mode 100644 test/vinyl/delete_run.result<br>> > create mode 100644 test/vinyl/delete_run.test.lua<br>> > create mode 100644 test/vinyl/force_recovery_true.lua<br>> ><br>> > diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c<br>> > index aa4bce9eb..a8abaf031 100644<br>> > --- a/src/box/vy_lsm.c<br>> > +++ b/src/box/vy_lsm.c<br>> > @@ -374,6 +374,9 @@ vy_lsm_recover_run(struct vy_lsm *lsm, struct vy_run_recovery_info *run_info,<br>> > lsm->space_id, lsm->index_id,<br>> > lsm->cmp_def, lsm->key_def,<br>> > lsm->disk_format, &lsm->opts) != 0)) {<br>> > + vy_log_tx_begin();<br>> > + vy_log_drop_run(run_info->id, 0);<br>> > + vy_log_tx_try_commit();<br>> > vy_run_unref(run);<br>> <br>> You can re-use vy_run_discard().<br><br>Yes, I agree.<br><br><br>> > return NULL;<br>> > }<br>> > @@ -426,8 +429,14 @@ vy_lsm_recover_slice(struct vy_lsm *lsm, struct vy_range *range,<br>> ><br>> > run = vy_lsm_recover_run(lsm, slice_info->run,<br>> > run_env, force_recovery);<br>> > - if (run == NULL)<br>> > + if (run == NULL) {<br>> > + if (force_recovery) {<br>> > + vy_log_tx_begin();<br>> > + vy_log_delete_slice(slice_info->id);<br>> > + vy_log_tx_try_commit();<br>> > + }<br>> > goto out;<br>> > + }<br>> ><br>> > slice = vy_slice_new(slice_info->id, run, begin, end, lsm->cmp_def);<br>> > if (slice == NULL)<br>> > @@ -486,9 +495,11 @@ vy_lsm_recover_range(struct vy_lsm *lsm,<br>> > rlist_foreach_entry_reverse(slice_info, &range_info->slices, in_range) {<br>> > if (vy_lsm_recover_slice(lsm, range, slice_info,<br>> > run_env, force_recovery) == NULL) {<br>> > - vy_range_delete(range);<br>> > - range = NULL;<br>> > - goto out;<br>> > + if (force_recovery == false) {<br>> <br>> Nit: if (!force_recovery)<br>> <br>> Why do we delete slice if force_recovery is enabled,<br>> meanwhile delete range/run when force_recovery is disabled?<br><br>Slice is a reference to a run containing a key range and it’s stored only in the metadata log.<br>So, if run file is deleted and we restart server with force_recovery=true we have to delete<br>the corresponding slice to ignore the file loss. <br>Range is an LSM-subtree, so if we want to get the rest of the subtree <span lang="en">at any cost (force_recovery=true)<br>we don't have to delete the range.</span><br><br>> Could you comment these test cases explaining what's going on here?<br> <br>> > +test_run:cmd("create server test with script='vinyl/force_recovery_true.lua'")<br>> > +test_run:cmd("start server test with args='1'")<br>> > +test_run:cmd("switch test")<br>> > +<br>> > +test = box.schema.space.create('test', {engine='vinyl'})<br>> > +_ = test:create_index('pk')<br>> > +for i = 1, 10 do test:insert{i, i + 100} box.snapshot() end<br>> > +test:select()<br>> > +<br>> > +fio = require ('fio')<br>> > +for _, f in pairs(fio.glob(box.cfg.vinyl_dir .. '/' .. test.id .. '/0/*26*')) do fio.unlink(f) end<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('restart server test with args="1"')<br>> > +test_run:cmd('switch test')<br>> > +box.cfg.force_recovery<br>> > +box.space.test:select()<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('restart server test with args="0"')<br>> > +test_run:cmd('switch test')<br>> > +box.cfg.force_recovery<br>> > +box.space.test:select()<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('stop server test')<br>> > +test_run:cmd('cleanup server test')<br>> > +test_run:cmd('delete server test')<br>> > +<br>> > +-- Test 2<br>> > +test_run = require('test_run').new()<br>> > +test_run:cmd("create server test with script='vinyl/force_recovery_true.lua'")<br>> > +test_run:cmd("start server test with args='1'")<br>> > +test_run:cmd("switch test")<br>> > +<br>> > +test = box.schema.space.create('test', {engine='vinyl'})<br>> > +_ = test:create_index('pk')<br>> > +_ = test:insert{1, "123"}<br>> > +test:select()<br>> > +box.snapshot()<br>> > +<br>> > +fio = require ('fio')<br>> > +for _, f in pairs(fio.glob(box.cfg.vinyl_dir .. '/' .. test.id .. '/0/*.index')) do fio.unlink(f) end<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('restart server test with args="1"')<br>> > +test_run:cmd('switch test')<br>> > +box.space.test:select()<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('restart server test with args="0"')<br>> > +test_run:cmd('switch test')<br>> > +box.space.test:select()<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('stop server test')<br>> > +test_run:cmd('cleanup server test')<br>> > +test_run:cmd('delete server test')<br>> > +<br>> > +-- Test3<br>> > +<br>> > +test_run = require('test_run').new()<br>> > +test_run:cmd("create server test with script='vinyl/force_recovery_true.lua'")<br>> > +test_run:cmd("start server test")<br>> > +test_run:cmd("switch test")<br>> > +<br>> > +test = box.schema.space.create('test', {engine='vinyl'})<br>> > +_ = test:create_index('pk')<br>> > +_ = test:insert{1, "123"}<br>> > +test:select(1)<br>> > +box.snapshot()<br>> > +<br>> > +fio = require ('fio')<br>> > +for _, f in pairs(fio.glob(box.cfg.vinyl_dir .. '/' .. test.id .. '/0/*.run')) do fio.unlink(f) end<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('restart server test with args="1"')<br>> > +test_run:cmd('switch test')<br>> > +box.space.test:select()<br>> > +<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('restart server test with args="0"')<br>> > +test_run:cmd('switch test')<br>> > +box.cfg.force_recovery<br>> > +box.space.test:select()<br>> > +test_run:cmd('switch default')<br>> > +test_run:cmd('stop server test')<br>> > +test_run:cmd('cleanup server test')<br>> > +test_run:cmd('delete server test')<br>> > diff --git a/test/vinyl/force_recovery_true.lua b/test/vinyl/force_recovery_true.lua<br>> > new file mode 100644<br>> > index 000000000..53414f22c<br>> > --- /dev/null<br>> > +++ b/test/vinyl/force_recovery_true.lua<br>> > @@ -0,0 +1,11 @@<br>> > +#!/usr/bin/env tarantool<br>bash: !/usr/bin/env: event not found<br>> > +<br>> > +local fr = tonumber(arg[1])<br>> > +box.cfg ({<br>> > + listen = os.getenv("LISTEN"),<br>> > + replication = os.getenv("MASTER"),<br>> > + vinyl_memory = 128 * 1024 * 1024,<br>> > + force_recovery = (fr == 1),<br>> > +})<br>> > +<br>> > +require('console').listen(os.getenv('ADMIN'))<br>> > --<br>> > 2.17.1<br>> ><br><br><br><br>In these tests I make some snapshots and delete .run and .index files. <br>Then I recover server with force_reovery = true,<br> and in the end I restart serverwith force_recovery = false to make sure that<br>the server will start up<br><br>At the first test I delete only one .run file one .index file.<br>The second test deletes all .index files and the third tests deletes all .run files.<br><br>branch: <a href="https://github.com/maksimkulis/tarantool/tree/maksimkulis/recover-after-deletion-run-files">https://github.com/maksimkulis/tarantool/tree/maksimkulis/recover-after-deletion-run-files</a> <br><br><blockquote style="border-left:1px solid #0857A6;margin:10px;padding:0 0 0 10px;">
Среда, 15 января 2020, 23:38 +03:00 от Nikita Pettik <<a href="//e.mail.ru/compose/?mailto=mailto%3akorablev@tarantool.org" target="_blank" rel=" noopener noreferrer">korablev@tarantool.org</a>>:<br>
<br>
<div id="">
<div class="js-helper_mailru_css_attribute_postfix_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix_mailru_css_attribute_postfix">
<style></style>
<div>
<div id="style_15791207391193514257_BODY_mailru_css_attribute_postfix_mailru_css_attribute_postfix">On 13 Jan 14:27, Maksim Kulis wrote:<br>
> While deletion .run files and recover with force_recovery=true we have to delete some logs which are responsible for deleted files so that we can recover later with force_recovery=false.<br>
<br>
Please, limit commit message with 80 characters per line.<br>
<br>
Is there any issue corresponding to the problem you are solving?<br>
<br>
> ---<br>
> src/box/vy_lsm.c | 19 +-<br>
> test/vinyl/delete_run.result | 302 +++++++++++++++++++++++++++++<br>
> test/vinyl/delete_run.test.lua | 93 +++++++++<br>
> test/vinyl/force_recovery_true.lua | 11 ++<br>
> 4 files changed, 421 insertions(+), 4 deletions(-)<br>
> create mode 100644 test/vinyl/delete_run.result<br>
> create mode 100644 test/vinyl/delete_run.test.lua<br>
> create mode 100644 test/vinyl/force_recovery_true.lua<br>
> <br>
> diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c<br>
> index aa4bce9eb..a8abaf031 100644<br>
> --- a/src/box/vy_lsm.c<br>
> +++ b/src/box/vy_lsm.c<br>
> @@ -374,6 +374,9 @@ vy_lsm_recover_run(struct vy_lsm *lsm, struct vy_run_recovery_info *run_info,<br>
> lsm->space_id, lsm->index_id,<br>
> lsm->cmp_def, lsm->key_def,<br>
> lsm->disk_format, &lsm->opts) != 0)) {<br>
> + vy_log_tx_begin();<br>
> + vy_log_drop_run(run_info->id, 0);<br>
> + vy_log_tx_try_commit();<br>
> vy_run_unref(run);<br>
<br>
You can re-use vy_run_discard().<br>
<br>
> return NULL;<br>
> }<br>
> @@ -426,8 +429,14 @@ vy_lsm_recover_slice(struct vy_lsm *lsm, struct vy_range *range,<br>
> <br>
> run = vy_lsm_recover_run(lsm, slice_info->run,<br>
> run_env, force_recovery);<br>
> - if (run == NULL)<br>
> + if (run == NULL) {<br>
> + if (force_recovery) {<br>
> + vy_log_tx_begin();<br>
> + vy_log_delete_slice(slice_info->id);<br>
> + vy_log_tx_try_commit();<br>
> + }<br>
> goto out;<br>
> + }<br>
> <br>
> slice = vy_slice_new(slice_info->id, run, begin, end, lsm->cmp_def);<br>
> if (slice == NULL)<br>
> @@ -486,9 +495,11 @@ vy_lsm_recover_range(struct vy_lsm *lsm,<br>
> rlist_foreach_entry_reverse(slice_info, &range_info->slices, in_range) {<br>
> if (vy_lsm_recover_slice(lsm, range, slice_info,<br>
> run_env, force_recovery) == NULL) {<br>
> - vy_range_delete(range);<br>
> - range = NULL;<br>
> - goto out;<br>
> + if (force_recovery == false) {<br>
<br>
Nit: if (!force_recovery)<br>
<br>
Why do we delete slice if force_recovery is enabled,<br>
meanwhile delete range/run when force_recovery is disabled?<br>
<br>
> + vy_range_delete(range);<br>
> + range = NULL;<br>
> + goto out;<br>
> + }<br>
> }<br>
> }<br>
> vy_lsm_add_range(lsm, range);<br>
> diff --git a/test/vinyl/delete_run.test.lua b/test/vinyl/delete_run.test.lua<br>
> new file mode 100644<br>
> index 000000000..1e8ed39ed<br>
> --- /dev/null<br>
> +++ b/test/vinyl/delete_run.test.lua<br>
> @@ -0,0 +1,93 @@<br>
> +test_run = require('test_run').new()<br>
> +fiber = require('fiber')<br>
> +<br>
> +-- Test 1<br>
<br>
Could you comment these test cases explaining what's going on here?<br>
<br>
> +test_run:cmd("create server test with script='vinyl/force_recovery_true.lua'")<br>
> +test_run:cmd("start server test with args='1'")<br>
> +test_run:cmd("switch test")<br>
> +<br>
> +test = box.schema.space.create('test', {engine='vinyl'})<br>
> +_ = test:create_index('pk')<br>
> +for i = 1, 10 do test:insert{i, i + 100} box.snapshot() end<br>
> +test:select()<br>
> +<br>
> +fio = require ('fio')<br>
> +for _, f in pairs(fio.glob(box.cfg.vinyl_dir .. '/' .. test.id .. '/0/*26*')) do fio.unlink(f) end<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('restart server test with args="1"')<br>
> +test_run:cmd('switch test')<br>
> +box.cfg.force_recovery<br>
> +box.space.test:select()<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('restart server test with args="0"')<br>
> +test_run:cmd('switch test')<br>
> +box.cfg.force_recovery<br>
> +box.space.test:select()<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('stop server test')<br>
> +test_run:cmd('cleanup server test')<br>
> +test_run:cmd('delete server test')<br>
> +<br>
> +-- Test 2<br>
> +test_run = require('test_run').new()<br>
> +test_run:cmd("create server test with script='vinyl/force_recovery_true.lua'")<br>
> +test_run:cmd("start server test with args='1'")<br>
> +test_run:cmd("switch test")<br>
> +<br>
> +test = box.schema.space.create('test', {engine='vinyl'})<br>
> +_ = test:create_index('pk')<br>
> +_ = test:insert{1, "123"}<br>
> +test:select()<br>
> +box.snapshot()<br>
> +<br>
> +fio = require ('fio')<br>
> +for _, f in pairs(fio.glob(box.cfg.vinyl_dir .. '/' .. test.id .. '/0/*.index')) do fio.unlink(f) end<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('restart server test with args="1"')<br>
> +test_run:cmd('switch test')<br>
> +box.space.test:select()<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('restart server test with args="0"')<br>
> +test_run:cmd('switch test')<br>
> +box.space.test:select()<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('stop server test')<br>
> +test_run:cmd('cleanup server test')<br>
> +test_run:cmd('delete server test')<br>
> +<br>
> +-- Test3<br>
> +<br>
> +test_run = require('test_run').new()<br>
> +test_run:cmd("create server test with script='vinyl/force_recovery_true.lua'")<br>
> +test_run:cmd("start server test")<br>
> +test_run:cmd("switch test")<br>
> +<br>
> +test = box.schema.space.create('test', {engine='vinyl'})<br>
> +_ = test:create_index('pk')<br>
> +_ = test:insert{1, "123"}<br>
> +test:select(1)<br>
> +box.snapshot()<br>
> +<br>
> +fio = require ('fio')<br>
> +for _, f in pairs(fio.glob(box.cfg.vinyl_dir .. '/' .. test.id .. '/0/*.run')) do fio.unlink(f) end<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('restart server test with args="1"')<br>
> +test_run:cmd('switch test')<br>
> +box.space.test:select()<br>
> +<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('restart server test with args="0"')<br>
> +test_run:cmd('switch test')<br>
> +box.cfg.force_recovery<br>
> +box.space.test:select()<br>
> +test_run:cmd('switch default')<br>
> +test_run:cmd('stop server test')<br>
> +test_run:cmd('cleanup server test')<br>
> +test_run:cmd('delete server test')<br>
> diff --git a/test/vinyl/force_recovery_true.lua b/test/vinyl/force_recovery_true.lua<br>
> new file mode 100644<br>
> index 000000000..53414f22c<br>
> --- /dev/null<br>
> +++ b/test/vinyl/force_recovery_true.lua<br>
> @@ -0,0 +1,11 @@<br>
> +#!/usr/bin/env tarantool<br>
> +<br>
> +local fr = tonumber(arg[1])<br>
> +box.cfg ({<br>
> + listen = os.getenv("LISTEN"),<br>
> + replication = os.getenv("MASTER"),<br>
> + vinyl_memory = 128 * 1024 * 1024,<br>
> + force_recovery = (fr == 1),<br>
> +})<br>
> +<br>
> +require('console').listen(os.getenv('ADMIN'))<br>
> -- <br>
> 2.17.1<br>
> <br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Maxim Kulis<br>
</div></div>
</div>
</div>
</div></div>
<base target="_self" href="https://e.mail.ru/">
</div>
</div></BODY></HTML>