From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH] vinyl: fail transaction immediately if it does not fit in memory Date: Sun, 1 Apr 2018 15:36:34 +0300 Message-Id: <903b6841e80fbe20f77f35d7bde24966bafc2d6f.1522584682.git.vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: If the size of a transaction is greater than the configured memory limit (box.cfg.vinyl_memory), the transaction will hang on commit for 60 seconds (box.cfg.vinyl_timeout) and then fail with the following error message: Timed out waiting for Vinyl memory quota This is confusing. Let's fail such transactions immediately with OutOfMemory error. Closes #3291 --- https://github.com/tarantool/tarantool/issues/3291 https://github.com/tarantool/tarantool/commits/gh-3291-vy-fail-huge-tx-early src/box/vinyl.c | 10 +++++++ test/vinyl/quota_timeout.result | 55 ++++++++++++++++++++++++++------------- test/vinyl/quota_timeout.test.lua | 34 +++++++++++++++--------- 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 8abfaac9..d2cd61f0 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -2325,6 +2325,16 @@ vinyl_engine_prepare(struct engine *engine, struct txn *txn) return -1; /* + * The configured memory limit will never allow us to commit + * this transaction. Fail early. + */ + if (tx->write_size > env->quota.limit) { + diag_set(OutOfMemory, tx->write_size, + "lsregion", "vinyl transaction"); + return -1; + } + + /* * Do not abort join/subscribe on quota timeout - replication * is asynchronous anyway and there's box.info.replication * available for the admin to track the lag so let the applier diff --git a/test/vinyl/quota_timeout.result b/test/vinyl/quota_timeout.result index 1e66d392..e4bced02 100644 --- a/test/vinyl/quota_timeout.result +++ b/test/vinyl/quota_timeout.result @@ -108,7 +108,7 @@ box.error.injection.set('ERRINJ_VY_RUN_WRITE_TIMEOUT', 0) --- - ok ... -s:drop() +s:truncate() --- ... box.snapshot() @@ -116,11 +116,36 @@ box.snapshot() - ok ... -- --- Check that exceeding quota triggers dump of all spaces. +-- Check that exceeding quota doesn't hang the scheduler +-- in case there's nothing to dump. -- -box.cfg{vinyl_timeout=0.01} +-- The following operation should fail instantly irrespective +-- of the value of 'vinyl_timeout' (gh-3291). +-- +box.info.vinyl().quota.used == 0 +--- +- true +... +box.cfg{vinyl_timeout = 9000} +--- +... +pad = string.rep('x', box.cfg.vinyl_memory) +--- +... +_ = s:auto_increment{pad} +--- +- error: Failed to allocate 1048615 bytes in lsregion for vinyl transaction +... +s:drop() +--- +... +box.snapshot() --- +- ok ... +-- +-- Check that exceeding quota triggers dump of all spaces. +-- s1 = box.schema.space.create('test1', {engine = 'vinyl'}) --- ... @@ -133,34 +158,28 @@ s2 = box.schema.space.create('test2', {engine = 'vinyl'}) _ = s2:create_index('pk') --- ... -_ = s1:auto_increment{} +pad = string.rep('x', 64) --- ... -box.info.vinyl().quota.used +_ = s1:auto_increment{pad} --- -- 49186 ... -pad = string.rep('x', box.cfg.vinyl_memory) +s1.index.pk:info().memory.bytes > 0 --- +- true ... -_ = s2:auto_increment{pad} +pad = string.rep('x', box.cfg.vinyl_memory - string.len(pad)) --- -- error: Timed out waiting for Vinyl memory quota ... -while box.info.vinyl().quota.used > 0 do fiber.sleep(0.01) end +_ = s2:auto_increment{pad} --- ... -box.info.vinyl().quota.used +while s1.index.pk:info().disk.dump.count == 0 do fiber.sleep(0.01) end --- -- 0 ... --- --- Check that exceeding quota doesn't hang the scheduler --- in case there's nothing to dump. --- -s2:auto_increment{pad} +s1.index.pk:info().memory.bytes == 0 --- -- error: Timed out waiting for Vinyl memory quota +- true ... test_run:cmd('switch default') --- diff --git a/test/vinyl/quota_timeout.test.lua b/test/vinyl/quota_timeout.test.lua index ed5ba79d..c3d17b44 100644 --- a/test/vinyl/quota_timeout.test.lua +++ b/test/vinyl/quota_timeout.test.lua @@ -50,33 +50,41 @@ test_run:cmd("clear filter") box.error.injection.set('ERRINJ_VY_RUN_WRITE_TIMEOUT', 0) +s:truncate() +box.snapshot() + +-- +-- Check that exceeding quota doesn't hang the scheduler +-- in case there's nothing to dump. +-- +-- The following operation should fail instantly irrespective +-- of the value of 'vinyl_timeout' (gh-3291). +-- +box.info.vinyl().quota.used == 0 +box.cfg{vinyl_timeout = 9000} +pad = string.rep('x', box.cfg.vinyl_memory) +_ = s:auto_increment{pad} + s:drop() box.snapshot() -- -- Check that exceeding quota triggers dump of all spaces. -- -box.cfg{vinyl_timeout=0.01} - s1 = box.schema.space.create('test1', {engine = 'vinyl'}) _ = s1:create_index('pk') s2 = box.schema.space.create('test2', {engine = 'vinyl'}) _ = s2:create_index('pk') -_ = s1:auto_increment{} -box.info.vinyl().quota.used +pad = string.rep('x', 64) +_ = s1:auto_increment{pad} +s1.index.pk:info().memory.bytes > 0 -pad = string.rep('x', box.cfg.vinyl_memory) +pad = string.rep('x', box.cfg.vinyl_memory - string.len(pad)) _ = s2:auto_increment{pad} -while box.info.vinyl().quota.used > 0 do fiber.sleep(0.01) end -box.info.vinyl().quota.used - --- --- Check that exceeding quota doesn't hang the scheduler --- in case there's nothing to dump. --- -s2:auto_increment{pad} +while s1.index.pk:info().disk.dump.count == 0 do fiber.sleep(0.01) end +s1.index.pk:info().memory.bytes == 0 test_run:cmd('switch default') test_run:cmd("stop server test") -- 2.11.0