From: Nikita Pettik <korablev@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] vinyl: bump dump_generation in case scheduler doesn't catch up with DDL Date: Fri, 3 Apr 2020 01:25:35 +0000 [thread overview] Message-ID: <20200403012535.GA946@tarantool.org> (raw) In-Reply-To: <c19975dd-3905-c83a-3421-ba85b5483eab@tarantool.org> On 03 Apr 02:21, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > See 3 comments below. > > On 31/03/2020 02:47, Nikita Pettik wrote: > > It may turn out that dump_generation does not catch up with current > > generation and no other dump tasks are in progress. This may happen > > dump process is throttled due to errors. In this case generation is > > bumped but dump_generation is not (since dump is not completed). In > > turn, throttling opens a window for DDL operations. For instance, index > > dropping and creation of new one results in mentioned situation: > > > > box.snapshot() -- fails for some reason; next attempt at dumping will be > > -- taken in one second. > > s:drop() -- drop index to be dumped > > s = box.schema.space.create('test', {engine = 'vinyl'}) > > -- create new one (its mem generation is greater than scheduler's one) > > i = s:create_index('pk') > > > > Closes #4821 > > --- > > 1. Add a changelog please. Will do. But firstly, I'm going to introduce more sophisticated tests involving creating, filling and deleting several LSMs and meanwhile random dump fails (to make sure that provided fix works in all other cases as well). > > Note that current fix is only workaround amd doesn't pretend to be > > the most proper one. Alternatively, we can rollback scheduler > > generation in case dump is failed and index to be dumped is dropped. > > 2. Not sure this is possible. At least I don't see a way. The dump is > failed before the index is dropped. And only on a next dump you see > that there is a problem. You can try changing the generation in the > index drop, if you see, that its generation matches the dump generation. > > But it looks like encapsulation violation if you do it straight forward. > vy_lsm works with the scheduler only via its API. If you go this way, > we need a method like vy_scheduler_lsm_is_dropped(). It would check that > if the LSM's generation = dump_generation, we should recalculate it > not counting this tree. > > However I am ok with both solutions. Up to you. > > > Branch: https://github.com/tarantool/tarantool/tree/np/gh-4821-scheduler-gen-assert > > Issue: https://github.com/tarantool/tarantool/issues/4821 > > > diff --git a/test/vinyl/gh-4821-ddl-during-throttled-dump.result b/test/vinyl/gh-4821-ddl-during-throttled-dump.result > > new file mode 100644 > > index 000000000..eee031101 > > 3. I rolledback the patch except the test, and it passed anyway. > The example in the ticket crashes on my machine, but the test > does not. Thanks for noting, I'll deal with it. > Why not to take the test from the ticket as is? With a small > change of throttling timeout using ERRINJ_VY_SCHED_TIMEOUT, to > make it fail faster than 1 sec. But I took test from the ticket except for removing VY_RUN_WRITE_DELAY injection (it is not vital here).
next prev parent reply other threads:[~2020-04-03 1:25 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-31 0:47 Nikita Pettik 2020-04-03 0:21 ` Vladislav Shpilevoy 2020-04-03 1:25 ` Nikita Pettik [this message] 2020-06-10 15:31 Aleksandr Lyapunov 2020-06-10 21:41 ` Nikita Pettik 2020-06-11 1:40 ` Nikita Pettik
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=20200403012535.GA946@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] vinyl: bump dump_generation in case scheduler doesn'\''t catch up with DDL' \ /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