From: Aleksandr Lyapunov <alyapunov@tarantool.org> To: Nikita Pettik <korablev@tarantool.org>, tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2] vinyl: restart read iterator in case of rolled back WAL Date: Tue, 16 Jun 2020 15:10:38 +0300 [thread overview] Message-ID: <4a83c68d-2dfd-23a8-97cd-5a429639dc3c@tarantool.org> (raw) In-Reply-To: <50a25fbae907f1b0d5406fb2e78d40d3e42a8a8d.1591029888.git.korablev@tarantool.org> Thanks for the patch! See my 2 comments below: On 6/1/20 7:46 PM, Nikita Pettik wrote: > > + if (vy_mem_tree_iterator_is_invalid(&src->mem_iterator.curr_pos)) { > + assert(src->mem_iterator.curr.stmt == NULL); > + return 1; > + } I'm afraid that the iterator will not always be invalid in the given case. As I see, if a mem holds any older tuple with the same key (e.g. older version of that tuple), the restoration routine will find the older tuple with the non-invalid iterator. I also think that mem_restore must handle all the possible data changes by itself without concern of read_iterator. > - if (vy_read_iterator_restore_mem(itr, &next) != 0) > + int rc = vy_read_iterator_restore_mem(itr, &next); > + if (rc < 0) > return -1; > + if (rc > 0) { > + vy_read_iterator_restore(itr); > + goto restart; > + } The read iterator was rewritten several times and still have at least several bugs. I think we should admit that we cannot support such a complicated solution. How about some stupid solution: if ANY change has been happened during yield - restart advancing?
next prev parent reply other threads:[~2020-06-16 12:10 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-01 16:46 Nikita Pettik 2020-06-01 17:40 ` Vladislav Shpilevoy 2020-06-19 12:34 ` Nikita Pettik 2020-06-20 16:33 ` Vladislav Shpilevoy 2020-06-16 12:10 ` Aleksandr Lyapunov [this message] 2020-06-19 12:24 ` Nikita Pettik 2020-06-19 12:42 ` Konstantin Osipov 2020-06-23 5:15 ` Aleksandr Lyapunov 2020-06-23 11:08 ` Konstantin Osipov 2020-06-19 13:01 ` Aleksandr Lyapunov 2020-06-24 13:41 ` Nikita Pettik 2020-06-20 16:33 ` Vladislav Shpilevoy
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=4a83c68d-2dfd-23a8-97cd-5a429639dc3c@tarantool.org \ --to=alyapunov@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2] vinyl: restart read iterator in case of rolled back WAL' \ /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