From 2dfcb896ea55b0407c80e3dc35ed542d04ba0a6c Mon Sep 17 00:00:00 2001 From: Magomed Kostoev Date: Fri, 15 Sep 2023 12:50:11 +0300 Subject: [PATCH] test: align space_index_test with the transactional DDL If the tarantool/tarantool#8751 is merged, the space drop is transactional, so it only yields once on the space drop operation finish. This causes the test fail [1] because of unexpected state in the expirationd task. This patch fixes the problem by wrapping space drop and recreation into a transaction. More informative description of what the issue is: Consider the following facts: a. If the PR mentioned above is merged, the space drop yields on the full DDL operation end. After that point, the space is dropped entirely (no indexes or entries in the `_space` space exist). b. If the expirationd task's `atomic_iteration` option is set to false (which is the default used in the test), each tuple drop causes the expirationd task fiber yield. So here's what happens if the fact 'a' is false (the PR mentioned above is not merged): 1. The test fiber creates the expirationd task and yields into it. 2. The expirationd task drops the first tuple from the given space and yields back to the test fiber. 3. The test fiber does the `space:drop()`, which is not atomic, so 4. the drop yields on the primary index drop. Control is returned to the expirationd task. 5. The expirationd task: 1. Finishes the space iteration (there was only one tuple in the given space) 2. Uses the space (`box.space[task.space].engine == "vinyl"`) we wanted to drop in the test fiber. 3. Goes sleep, which yields to the test fiber. 6. The test fiber continues the space drop, finishes it (the next yields happening during the space drop do not return the control back to the expirationd task though, because it sleeps and it's sleep timer hasn't expired yet). 7. The test fiber recreates the space, and finally, when it's time to switch back to the expirationd task, the space exists again like no space drop happened. But if the space drop is atomic things change starting from the step 3: the space drop only yields on the whole DDL transaction commit. After that point, the space does not exist anymore. So: 4. The atomic space drop yields to the expirationd task on commit. 5. The expirationd task: 1. Finishes the iteration and checks if the space's engine is vinyl in order to update the vinyl_assumed_space_len, but the space does not exist anymore, so it fails. This causes the expirationd task restart. The last mentioned step increments the restart counter so the test check fails. In order to mitigate that the patch wraps the space drop and recreation into a transaction, so the test fiber only yields when all the required changes are applied, so the expirationd task does not see the dropped space and only works with the original or recreated space. The tranactioning is only done for Tarantool version 2.2.1 or newer because in the version the single-yield transactional DDL had been introduced. 1. https://github.com/tarantool/tarantool/actions/runs/6186475380/job/16795081554 --- CHANGELOG.md | 4 ++++ test/helper.lua | 13 +++++++++++++ test/unit/space_index_test.lua | 30 ++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f101cd2..741bf32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed +- Updated the 'space_index_test.lua' to drop and recreate the test space + atomically. This prevents the space access failure in the expirationd + task fiber if the `space:drop` function is transactional (#157). + ### Fixed ## 1.5.0 - 2023-08-23 diff --git a/test/helper.lua b/test/helper.lua index 7488814..7a9e646 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -287,6 +287,19 @@ function helpers.memtx_func_index_is_supported() (major >= 3) end +function helpers.single_yield_transactional_ddl_is_supported() + local major, minor, patch = helpers.tarantool_version() + + -- The issue: https://github.com/tarantool/tarantool/issues/4083 + -- + -- A limited transactional DDL support has been introduced in 2.2.1, it + -- allows to wrap a single-yield DDL statement set into a transaction if + -- the yielding statement is the first in the transaction. + return (major == 2 and minor == 2 and patch >= 1) or + (major == 2 and minor >= 3) or + (major >= 3) +end + function helpers.error_function() error("error function call") end diff --git a/test/unit/space_index_test.lua b/test/unit/space_index_test.lua index 45947a7..bc35b36 100644 --- a/test/unit/space_index_test.lua +++ b/test/unit/space_index_test.lua @@ -348,9 +348,39 @@ function g.test_not_stop_after_drop_and_recreate(cg) end) helpers.iteration_result = {} + + -- This should be atomic to prevent yielding into the expirationd task with + -- partially applied changes (when the old space is dropped but the new one + -- is not created yet). + -- + -- The space drop is transactional if the tarantool/taratool#8751 is merged, + -- so it yields on commit. After that point the space is dropped. + -- + -- The expirationd task yields on each tuple drop if the task does not have + -- the atomic_iteration option set. + -- + -- So without this begin/commit guard the following situation is possible: + -- 1. The expirationd task has started. + -- 2. The task removes one tuple, which causes yield. + -- 3. The current fiber drops the space and yields to the expirationd task. + -- 4. The expirationd task finishes the iteration and checks if the space + -- is vinyl to update the vinyl_assumed_space_len, but the space does + -- not exist anymore, so it fails causing the task restart. + + -- Since the test is also executed against old versions of Tarantool, let's + -- only wrap the space drop and recreation into transaction on the versions + -- that support at least single-yield transactional DDL. + if helpers.single_yield_transactional_ddl_is_supported() then + box.begin() + end + cg.case_space:drop() create_case_space(cg, space_name) + if helpers.single_yield_transactional_ddl_is_supported() then + box.commit() + end + helpers.retrying({}, function() t.assert_equals(helpers.iteration_result, {{2, "2"}}) end)