From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A6B85430408 for ; Thu, 13 Aug 2020 00:23:44 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: <02049f5a-702b-d2c7-6873-c5b26e9de005@tarantool.org> Date: Wed, 12 Aug 2020 23:23:42 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH V2] vinyl: rework upsert operation List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , tarantool-patches@dev.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.