Skip to content

Commit ae519ed

Browse files
committed
vinyl: do not reset cache link LSN on partial key lookup
Since commit e8109b2 ("vinyl: ignore cache chains with invisible DELETEs") each cache node stores the LSNs of the left and right links. They are used to ignore links that are invisible from the current read view (created after the read view was opened). We update the LSN not only when we link two nodes together but also when we update the node's boundary level. This is incorrect if the node is linked. For example, suppose we have an index over two fields that contains tuples {0,2} and {2,0} and there's a transaction that was sent to a read view. Now another transaction inserts {0,1} and deletes {0,2}, then selects all tuples. This results in creation of two interlinked cache nodes: {0,1} and {2,0}. The link is invisible from the read view: since we skipped DELETE{0,2}, its LSN equals the LSN of the DELETE statement, which is greater than the read view LSN. However, if we now select GT{1}, we will update the boundary level of the node storing {2,0} from 2 to 1 and reset its LSN to 0 because we didn't skip any DELETE statements. As a result, if the transaction operating in the read view tries to select GT{0,1}, it will find {2,0}, see that it's left-linked and the link LSN is 0, and return it right away, skipping {0,2}, which is visible from the read view. To fix this issue, let's skip the boundary level update if the node is linked. It doesn't make sense anyway because the boundary level is used only for unlinked nodes, see vy_cache_iterator_is_stop(). Follow-up tarantool#11079 Closes tarantool#11294 NO_DOC=bug fix
1 parent f001417 commit ae519ed

File tree

3 files changed

+59
-1
lines changed

3 files changed

+59
-1
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
## bugfix/vinyl
22

33
* Fixed a bug in the tuple cache when a transaction operating in a read view
4-
could skip a tuple deleted after the read view creation (gh-11079).
4+
could skip a tuple deleted after the read view creation (gh-11079, gh-11294).

src/box/vy_cache.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,10 +333,12 @@ vy_cache_add(struct vy_cache *cache, struct vy_entry curr,
333333
vy_cache_node_delete(cache->env, replaced);
334334
}
335335
if (direction > 0 &&
336+
!(node->flags & VY_CACHE_LEFT_LINKED) &&
336337
boundary_level < node->left_boundary_level) {
337338
node->left_boundary_level = boundary_level;
338339
node->left_lsn = link_lsn;
339340
} else if (direction < 0 &&
341+
!(node->flags & VY_CACHE_RIGHT_LINKED) &&
340342
boundary_level < node->right_boundary_level) {
341343
node->right_boundary_level = boundary_level;
342344
node->right_lsn = link_lsn;

test/vinyl-luatest/gh_11079_invisible_delete_caching_test.lua

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,3 +199,59 @@ g.test_deferred_delete = function(cg)
199199
{{60, 600}, {40, 400}, {20, 200}})
200200
end)
201201
end
202+
203+
g.test_partial_key_forward = function(cg)
204+
cg.server:exec(function()
205+
local fiber = require('fiber')
206+
207+
local s = box.schema.space.create('test', {engine = 'vinyl'})
208+
s:create_index('primary', {parts = {{1, 'unsigned'}, {2, 'unsigned'}}})
209+
210+
s:replace({0, 2})
211+
s:replace({2, 0})
212+
213+
box.begin()
214+
t.assert_equals(s:select(), {{0, 2}, {2, 0}})
215+
216+
local f = fiber.new(function()
217+
s:replace({0, 1})
218+
s:delete({0, 2})
219+
t.assert_equals(s:select(), {{0, 1}, {2, 0}})
220+
t.assert_equals(s:select({1}, {iterator = 'gt'}), {{2, 0}})
221+
end)
222+
f:set_joinable(true)
223+
t.assert_equals({f:join(5)}, {true})
224+
225+
t.assert_equals(s:select({}, {iterator = 'lt'}), {{2, 0}, {0, 2}})
226+
t.assert_equals(s:select({0, 1}, {iterator = 'gt'}), {{0, 2}, {2, 0}})
227+
box.commit()
228+
end)
229+
end
230+
231+
g.test_partial_key_backward = function(cg)
232+
cg.server:exec(function()
233+
local fiber = require('fiber')
234+
235+
local s = box.schema.space.create('test', {engine = 'vinyl'})
236+
s:create_index('primary', {parts = {{1, 'unsigned'}, {2, 'unsigned'}}})
237+
238+
s:replace({0, 2})
239+
s:replace({2, 0})
240+
241+
box.begin()
242+
t.assert_equals(s:select(), {{0, 2}, {2, 0}})
243+
244+
local f = fiber.new(function()
245+
s:replace({2, 1})
246+
s:delete({2, 0})
247+
t.assert_equals(s:select(), {{0, 2}, {2, 1}})
248+
t.assert_equals(s:select({1}, {iterator = 'lt'}), {{0, 2}})
249+
end)
250+
f:set_joinable(true)
251+
t.assert_equals({f:join(5)}, {true})
252+
253+
t.assert_equals(s:select({}, {iterator = 'gt'}), {{0, 2}, {2, 0}})
254+
t.assert_equals(s:select({2, 1}, {iterator = 'lt'}), {{2, 0}, {0, 2}})
255+
box.commit()
256+
end)
257+
end

0 commit comments

Comments
 (0)