Re: [Tarantool-patches] [PATCH V2] vinyl: rework upsert operation

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

> Closes #5105
> ---
> Issues:
> https://github.com/tarantool/tarantool/issues/5105
> 
> @ChangeLog:
>    - upserts now follow associative property: result of several upserts
>      doesn't depend on the order of their application (gh-5105);

In my previous review I tried to explain that any squash now won't work, if we want
to fix 5105. It does not matter if you check for type match. Because still there are
overflows. Consider this test:

	box.cfg{}
	format = {}
	format[1] = {name = 'f1', type = 'unsigned'}
	format[2] = {name = 'f2', type = 'unsigned'}
	s = box.schema.space.create('test', {engine = 'vinyl', format = format})
	_ = s:create_index('pk')
	uint_max = 18446744073709551615ULL
	s:replace{1, uint_max - 2}
	box.snapshot()

	s:upsert({1, 0}, {{'+', 2, 1}})
	s:upsert({1, 0}, {{'+', 2, 1}})
	s:upsert({1, 0}, {{'+', 2, 1}})
	box.snapshot()
	s:select()

	---
	- - [1, 18446744073709551614]
	...

Last 2 upserts were ignored. Even though only the last one should have been.
Indeed, if I comment it out, the result becomes correct:

	box.cfg{}
	format = {}
	format[1] = {name = 'f1', type = 'unsigned'}
	format[2] = {name = 'f2', type = 'unsigned'}
	s = box.schema.space.create('test', {engine = 'vinyl', format = format})
	_ = s:create_index('pk')
	uint_max = 18446744073709551615ULL
	s:replace{1, uint_max - 2}
	box.snapshot()

	s:upsert({1, 0}, {{'+', 2, 1}})
	s:upsert({1, 0}, {{'+', 2, 1}})
	-- s:upsert({1, 0}, {{'+', 2, 1}})
	box.snapshot()
	s:select()

	---
	- - [1, 18446744073709551615]
	...

Also I tried to make it depend on order, not on being applied or not. And
used this test:

	box.cfg{}
	format = {}
	format[1] = {name = 'f1', type = 'unsigned'}
	format[2] = {name = 'f2', type = 'unsigned'}
	s = box.schema.space.create('test', {engine = 'vinyl', format = format})
	_ = s:create_index('pk')
	uint_max = 18446744073709551615ULL
	s:replace{1, uint_max - 2, 0}
	box.snapshot()

	s:upsert({1, 0, 0}, {{'+', 2, 1}})
	s:upsert({1, 0, 0}, {{'+', 2, 1}})
	-- I wanted to check, if +0.5 will prevent squashing, because
	-- it does not satisfy format. And that if I make it after all
	-- the other upserts, the bug will appear again.
	s:upsert({1, 0, 0}, {{'+', 2, 0.5}})
	s:upsert({1, 0, 0}, {{'+', 2, 1}})
	box.snapshot()
	s:select()

But got another bug instead:

	main/103/interactive I> vinyl checkpoint started
	main/105/checkpoint_daemon I> scheduled next checkpoint for Thu Aug 13 00:52:21 2020
	main/107/vinyl.scheduler I> 512/0: dump started
	vinyl.dump.0/103/task vy_stmt.c:174 E> ER_VINYL_MAX_TUPLE_SIZE: Failed to allocate 1347440769 bytes for tuple: tuple is too large. Check 'vinyl_max_tuple_size' configuration option.
	main/107/vinyl.scheduler vy_stmt.c:174 E> ER_VINYL_MAX_TUPLE_SIZE: Failed to allocate 1347440769 bytes for tuple: tuple is too large. Check 'vinyl_max_tuple_size' configuration option.
	main/107/vinyl.scheduler vy_scheduler.c:1292 E> 512/0: dump failed
	main/107/vinyl.scheduler vy_scheduler.c:2058 W> throttling scheduler for 1 second(s)
	snapshot/101/main I> saving snapshot `./00000000000000000008.snap.inprogress'
	snapshot/101/main I> done
	main/103/interactive vy_scheduler.c:740 E> vinyl checkpoint failed: Failed to allocate 1347440769 bytes for tuple: tuple is too large. Check 'vinyl_max_tuple_size' configuration option.
	---
	- error: 'Failed to allocate 1347440769 bytes for tuple: tuple is too large. Check
	    ''vinyl_max_tuple_size'' configuration option.'
	...

Seems the tuple is corrupted somewhere, because a few numbers clearly can't
occupy 1347440769 bytes.