Skip to content

Commit bde28f0

Browse files
committed
vinyl: fix crash in index drop if there is DML request reading from it
A DML request (insert, replace, update) can yield while reading from the disk in order to check unique constraints. In the meantime the index can be dropped. The DML request can't crash in this case thanks to commit d3e1236 ("vinyl: abort affected transactions when space is removed from cache"), but the DDL operation can because: - It unreferences the index in `alter_space_commit`, which may result in dropping the LSM tree with `vy_lsm_delete`. - `vy_lsm_delete` may yield in `vy_range_tree_free_cb` while waiting for disk readers to complete. - Yielding in commit triggers isn't allowed (crashes). We already fixed a similar issue when `index.get` crashed if raced with index drop, see commit 75f03a5 ("vinyl: fix crash if space is dropped while space.get is reading from it"). Let's fix this issue in the same way - by taking a reference to the LSM tree while checking unique constraints. To do that it's enough to move `vy_lsm_ref` from `vinyl_index_get` to `vy_get`. Also, let's replace `vy_slice_wait_pinned` with an assertion checking that the slice pin count is 0 in `vy_range_tree_free_cb` because `vy_lsm_delete` must not yield. Closes #10094 NO_DOC=bug fix
1 parent bc0daf9 commit bde28f0

File tree

4 files changed

+62
-11
lines changed

4 files changed

+62
-11
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
## bugfix/vinyl
2+
3+
* Fixed a bug when a DDL operation dropping a unique index could crash
4+
if performed concurrently with DML requests (gh-10094).

src/box/vinyl.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,11 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx,
14081408
* space.index.get({key}).
14091409
*/
14101410
assert(tx == NULL || tx->state == VINYL_TX_READY);
1411+
/*
1412+
* Make sure the LSM tree isn't deleted while we are
1413+
* reading from it.
1414+
*/
1415+
vy_lsm_ref(lsm);
14111416

14121417
int rc;
14131418
struct vy_entry partial, entry;
@@ -1423,15 +1428,15 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx,
14231428
* Use point lookup for a full key.
14241429
*/
14251430
if (tx != NULL && vy_tx_track_point(tx, lsm, key) != 0)
1426-
return -1;
1431+
goto fail;
14271432
if (vy_point_lookup(lsm, tx, rv, key, &partial) != 0)
1428-
return -1;
1433+
goto fail;
14291434
if (lsm->index_id > 0 && partial.stmt != NULL) {
14301435
rc = vy_get_by_secondary_tuple(lsm, tx, rv,
14311436
partial, &entry);
14321437
tuple_unref(partial.stmt);
14331438
if (rc != 0)
1434-
return -1;
1439+
goto fail;
14351440
} else {
14361441
entry = partial;
14371442
}
@@ -1457,7 +1462,7 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx,
14571462
vy_read_iterator_cache_add(&itr, entry);
14581463
vy_read_iterator_close(&itr);
14591464
if (rc != 0)
1460-
return -1;
1465+
goto fail;
14611466
out:
14621467
*result = entry.stmt;
14631468

@@ -1472,7 +1477,11 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx,
14721477
}
14731478
if (*result != NULL)
14741479
vy_stmt_counter_acct_tuple(&lsm->stat.get, *result);
1480+
vy_lsm_unref(lsm);
14751481
return 0;
1482+
fail:
1483+
vy_lsm_unref(lsm);
1484+
return -1;
14761485
}
14771486

14781487
/**
@@ -3862,14 +3871,8 @@ vinyl_index_get(struct index *index, const char *key,
38623871
tx = &tx_autocommit;
38633872
vy_tx_create(env->xm, tx);
38643873
}
3865-
/*
3866-
* Make sure the LSM tree isn't deleted while we are
3867-
* reading from it.
3868-
*/
3869-
vy_lsm_ref(lsm);
38703874
int rc = vy_get_by_raw_key(lsm, tx, vy_tx_read_view(tx),
38713875
key, part_count, ret);
3872-
vy_lsm_unref(lsm);
38733876
if (tx == &tx_autocommit)
38743877
vy_tx_destroy(tx);
38753878
if (rc != 0)

src/box/vy_lsm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ vy_range_tree_free_cb(vy_range_tree_t *t, struct vy_range *range, void *arg)
256256
(void)arg;
257257
struct vy_slice *slice;
258258
rlist_foreach_entry(slice, &range->slices, in_range)
259-
vy_slice_wait_pinned(slice);
259+
assert(slice->pin_count == 0);
260260
vy_range_delete(range);
261261
return NULL;
262262
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
local server = require('luatest.server')
2+
local t = require('luatest')
3+
4+
local g = t.group()
5+
6+
g.before_all(function(cg)
7+
t.tarantool.skip_if_not_debug()
8+
cg.server = server:new()
9+
cg.server:start()
10+
cg.server:exec(function()
11+
box.cfg{vinyl_cache = 0}
12+
end)
13+
end)
14+
15+
g.after_all(function(cg)
16+
cg.server:drop()
17+
end)
18+
19+
g.after_each(function(cg)
20+
cg.server:exec(function()
21+
if box.space.test ~= nil then
22+
box.space.test:drop()
23+
end
24+
box.error.injection.set('ERRINJ_VY_READ_PAGE_DELAY', false)
25+
end)
26+
end)
27+
28+
g.test_index_drop = function(cg)
29+
cg.server:exec(function()
30+
local fiber = require('fiber')
31+
local s = box.schema.create_space('test', {engine = 'vinyl'})
32+
s:create_index('pk')
33+
s:create_index('sk', {parts = {{2, 'unsigned'}}})
34+
s:insert{1, 10}
35+
box.snapshot()
36+
box.error.injection.set('ERRINJ_VY_READ_PAGE_DELAY', true)
37+
fiber.create(function()
38+
s:insert{2, 10}
39+
end)
40+
s.index.sk:drop()
41+
box.error.injection.set('ERRINJ_VY_READ_PAGE_DELAY', false)
42+
t.assert_equals(s:select({}, {fullscan = true}), {{1, 10}})
43+
end)
44+
end

0 commit comments

Comments
 (0)