Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja.osipov@gmail.com>
To: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 2/2] vinyl: skip invalid upserts during squash
Date: Thu, 14 May 2020 09:56:08 +0300	[thread overview]
Message-ID: <20200514065608.GA12581@atlas> (raw)
In-Reply-To: <20200514021155.GB18509@tarantool.org>

* Nikita Pettik <korablev@tarantool.org> [20/05/14 09:50]:
> On 14 Apr 01:12, Konstantin Osipov wrote:
> > * Nikita Pettik <korablev@tarantool.org> [20/04/14 00:57]:
> > > Instead of aborting merge sequence of upserts let's log appeared
> > > errors and skip upserts which can't be applied. It makes sense
> > > taking into consideration previous commit: now upsert statements which
> > > can't be applied may appear in mems/runs (previously squash operation
> > > might fail only due to OOM). As a result, if we didn't ignore invalid
> > > upserts, dump or compaction processes would not be able to finish (owing
> > > to inability to squash upserts).
> > 
> > We should log the upsert itself, base64 encoded. 
> 
> It is logged with following format (in vy_read_view_merge()):
> 
> say_info("upsert %s is not applied due to the error: %s",
>          vy_stmt_str(h->tuple), e->errmsg);
>          
> It is the function which is called during compaction to squash
> upserts.
> 
> Also, I've added logs during squash/apply of upserts in in-memory
> tree: see diffs in vy_tx_write() and vy_tx_set(). So that user gets
> the same error as when executing invalid upserts in memtx. However,
> I've found these logs a bit inconsistent: upserts are not always
> squashed/applied once they are inserted in tree. For instance:
> 
> s = box.schema.space.create('test', { engine = 'vinyl', field_count = 2 })
> pk = s:create_index('pk')
> s:replace{1, 1}
> s:upsert({1, 1}, {{'=', 3, 5}})
> main/101/interactive tuple.c:150 E> ER_EXACT_FIELD_COUNT: Tuple field count 3 does not match space field count 2 
> 
> This upsert is attepted to be optimized: in vy_lsm_commit_upsert()
> we try to apply it on replace statement, but we fail so error is logged
> (still upsert has been inserted in tree). Now, if we execute this
> upsert once again:
> 
> s:upsert({1, 1}, {{'=', 3, 5}})
> 
> error won't be logged, since previous upsert now resides in tree
> and we won't attempt at applying it (according to optimization's conditions).
> 
> Hence now I'm not sure that logging failed upsert squashes/applications
> is worth it when it comes for in-memory lsm level.

This is fine, the correct behaviour is not possible to define anyway, so best
effort on tarantool part is good enough.

If it becomes an issue we could create a local space which will
receive these upserts, instead of a log file, something like
_vy_failed_upserts, but I'd not expect it to be very useful.

> > Second, vy_history_apply may be called from many contexts - reads
> > and writes, for example. We should only log and skip during
> > compaction, not when reading, otherwise we'll spam the log file at
> > least.
> 
> We don't log anything in vy_history_apply(). So there are no logs
> during reads.

OK

> > Finally, let's double check there are no issues with the used
> > format - can it become obsolete by the time it's used, e.g. if
> > there is an online/non-blocking schema change that happened in tx
> > thread (compaction is running in the write thread)?
> 
> Upserts are supported only by primary index. Meanwhile vinyl does
> not support altering primary index of non-empty space. Am I missing
> something?

I mean the space format object itself. Squashing is happening in a
different thread, the 
> 

-- 
Konstantin Osipov, Moscow, Russia

  reply	other threads:[~2020-05-14  6:56 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-13 21:55 [Tarantool-patches] [PATCH 0/2] Validate result of upserts squash & skip invalid upserts which can't be applied Nikita Pettik
2020-04-13 21:55 ` [Tarantool-patches] [PATCH 1/2] vinyl: validate resulting tuple after upsert is applied Nikita Pettik
2020-06-22 19:28   ` Aleksandr Lyapunov
2020-04-13 21:55 ` [Tarantool-patches] [PATCH 2/2] vinyl: skip invalid upserts during squash Nikita Pettik
2020-04-13 22:12   ` Konstantin Osipov
2020-05-14  2:11     ` Nikita Pettik
2020-05-14  6:56       ` Konstantin Osipov [this message]
2020-05-19 19:10         ` Nikita Pettik
2020-05-19 19:39           ` Konstantin Osipov
2020-05-21  2:51             ` Nikita Pettik
2020-05-21  8:36               ` Konstantin Osipov
2020-05-01  0:31   ` Vladislav Shpilevoy
2020-05-14  2:21     ` Nikita Pettik
2020-05-14 21:32       ` Vladislav Shpilevoy
2020-05-19 18:18         ` Nikita Pettik
2020-05-20 22:13           ` Vladislav Shpilevoy
2020-05-26 21:33     ` Vladislav Shpilevoy
2020-05-27 20:05       ` Nikita Pettik
2020-05-29 21:47         ` Vladislav Shpilevoy
2020-06-01 19:24           ` Nikita Pettik
2020-05-20 22:13 ` [Tarantool-patches] [PATCH 0/2] Validate result of upserts squash & skip invalid upserts which can't be applied Vladislav Shpilevoy
2020-05-22  2:42   ` Nikita Pettik
2020-05-26 21:33     ` Vladislav Shpilevoy
2020-05-27 20:10       ` Nikita Pettik
2020-06-22 14:13     ` Aleksandr Lyapunov
2020-06-22 20:21       ` Nikita Pettik
2020-06-23 12:32         ` Aleksandr Lyapunov
2020-06-02 21:36 ` Vladislav Shpilevoy
2020-06-02 21:37   ` 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=20200514065608.GA12581@atlas \
    --to=kostja.osipov@gmail.com \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/2] vinyl: skip invalid upserts during squash' \
    /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