Re: [Tarantool-patches] [PATCH 2/2] vinyl: clean-up read views if *_build_history() fails

[Nikita Pettik <korablev@tarantool.org>]

On 11 Apr 00:32, Vladislav Shpilevoy wrote:
> 
> 
> On 10/04/2020 23:45, Nikita Pettik wrote:
> > On 10 Apr 15:47, Nikita Pettik wrote:
> > 
> > I've also found that vy_read_view_merge() can fail as well during
> > processing array of read views. Thus, we should clean-up unprocessed
> > read views as well. Diff:
> > 
> > diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
> > index 7d8c60d73..165b422fd 100644
> > --- a/src/box/vy_write_iterator.c
> > +++ b/src/box/vy_write_iterator.c
> > @@ -986,8 +986,11 @@ vy_write_iterator_build_read_views(struct vy_write_iterator *stream, int *count)
> >                 if (rv->history == NULL)
> >                         continue;
> >                 if (vy_read_view_merge(stream, hint, rv,
> > -                                      is_first_insert) != 0)
> > +                                      is_first_insert) != 0) {
> > +                       while (rv >= &stream->read_views[0]) {
> > +                               rv->history = NULL;
> > +                               rv--;
> > +                       }
> >                         goto error;
> 
> The same question as to the previous fix - shouldn't that be
> cleaned by the function, which created the mess, by
> vy_read_view_merge() internally?

Iteration over read views takes place in _build_read_views() so
to me it seems to be sensible to clean-up it there..
In fact, view_merge() does not create mess. Moreover, it doesn't
even have explicit access to array of read views (only via
stream->read_views[]) - read view to be processed is passed as a
separate parameter. Purpose of view_merge() is to merge statements,
so why it should clean-up read-views which belong to stream object?
All in all, I do not see much sense to move this clean-up to
read_view_merge(), only if you insist on this change.

> > +               }
> >                 assert(rv->history == NULL);
> >                 if (rv->tuple == NULL)
> >                         continue;
> >