osd: Roll back unstable writes of removed pools in the old store #106

Open
koolkhel wants to merge 1 commits from koolkhel/vitastor:removed-pools-v1 into master
Contributor

Description

Nothing stabilizes or rolls back unstable writes of a removed pool, its PGs are never peered again. The old store can't trim its journal past them, so the journal fills up and writes to all other pools on the same OSDs fail with -EAGAIN forever, restarts don't help.

Add a synchronous get_unstable_objects() which walks dirty_db of the old store, and roll back unstable writes of removed pools with it. The OSD only picks up a pool when it disappears from the pool configuration, or on start when the store has unstable writes of a pool that doesn't exist, and lists the store once the pool's PGs are removed and stopped. In normal operation it does nothing: PG changes only check that the set of removed pools is empty. The new store keeps such writes in its metadata without harm, so the OSD does nothing for it (blockstore_i::is_new()).

Tested:

  • test_blockstore_v1: new test_get_unstable_objects
  • osd_test: new test_rollback_removed_pool_on_start, test_rollback_removed_pool_online, test_rollback_removed_pool_idle (ordinary pool and PG changes don't list the store, the new store is never listed)
  • new tests/test_rm_pool_unstable.sh (old store): removes a pool with unstable writes online and while OSDs are down, then writes 4x the journal size to another pool. Without the patch it fails with "Pool removed online: writes to another pool got stuck"; with it every OSD logs "Rolled back 8 unstable object versions of removed pools" in both phases.

Not covered: an OSD whose journal is already completely full can't be healed, the rollback itself needs a journal sector.

Contributor Name

Yury Luneff (MIND Software LLC, https://mindsw.io)

CLA

### Description Nothing stabilizes or rolls back unstable writes of a removed pool, its PGs are never peered again. The old store can't trim its journal past them, so the journal fills up and writes to all other pools on the same OSDs fail with -EAGAIN forever, restarts don't help. Add a synchronous `get_unstable_objects()` which walks `dirty_db` of the old store, and roll back unstable writes of removed pools with it. The OSD only picks up a pool when it disappears from the pool configuration, or on start when the store has unstable writes of a pool that doesn't exist, and lists the store once the pool's PGs are removed and stopped. In normal operation it does nothing: PG changes only check that the set of removed pools is empty. The new store keeps such writes in its metadata without harm, so the OSD does nothing for it (`blockstore_i::is_new()`). Tested: - `test_blockstore_v1`: new `test_get_unstable_objects` - `osd_test`: new `test_rollback_removed_pool_on_start`, `test_rollback_removed_pool_online`, `test_rollback_removed_pool_idle` (ordinary pool and PG changes don't list the store, the new store is never listed) - new `tests/test_rm_pool_unstable.sh` (old store): removes a pool with unstable writes online and while OSDs are down, then writes 4x the journal size to another pool. Without the patch it fails with "Pool removed online: writes to another pool got stuck"; with it every OSD logs "Rolled back 8 unstable object versions of removed pools" in both phases. Not covered: an OSD whose journal is already completely full can't be healed, the rollback itself needs a journal sector. ### Contributor Name Yury Luneff (MIND Software LLC, https://mindsw.io) ### CLA - [x] I accept Vitastor CLA agreement: https://git.yourcmc.ru/vitalif/vitastor/src/branch/master/CLA-en.md
koolkhel added 1 commit 2026-10-26 20:54:42 +03:00
Nothing stabilizes or rolls back unstable writes of a removed pool, its
PGs are never peered again. The old store can't trim its journal past
them, so the journal fills up and writes to all other pools on the same
OSDs fail with -EAGAIN forever, restarts don't help.

Add a synchronous get_unstable_objects() which walks dirty_db of the old
store, and roll back unstable writes of objects outside of existing pools
with it: on start, and when a pool and its PG configuration are removed
and local PGs of the pool have stopped. The new store keeps such writes
in its metadata without harm, so it returns nothing.
vitalif requested changes 2026-10-27 00:28:12 +03:00
@@ -505,6 +505,10 @@ void osd_t::on_change_etcd_state_hook(std::map<std::string, etcd_kv_t> & changes
}
apply_pg_config();
}
if (pools || changes.find(st_cli->etcd_prefix+"/pg/config") != changes.end())
Owner

тут два раза чтоб не делать pgs =, можно выше pgs = 1 раз присвоить а тут только проверять

тут два раза чтоб не делать pgs =, можно выше pgs = 1 раз присвоить а тут только проверять
@@ -728,6 +732,7 @@ void osd_t::on_load_pgs_hook(bool success)
apply_pg_count();
apply_pg_config();
}
rollback_removed_pools();
Owner

тут делается rollback_(), а выше только check_(). как будто не совсем сбалансированно, надо по идее одно и то же триггерить, ну либо в одном месте где-то триггерить

тут делается rollback_(), а выше только check_(). как будто не совсем сбалансированно, надо по идее одно и то же триггерить, ну либо в одном месте где-то триггерить
@@ -476,0 +482,4 @@
{
for (auto & pc: st_cli->pool_config)
{
if (pc.second.exists)
Owner

а я чот не понял, т.е. это не срабатывает, если есть вообще хотя бы 1 живой пул? так это бессмысленно, нужно-то чтобы срабатывало для удалённых

а я чот не понял, т.е. это не срабатывает, если есть вообще хотя бы 1 живой пул? так это бессмысленно, нужно-то чтобы срабатывало для удалённых
@@ -476,0 +486,4 @@
{
continue;
}
// Wait until PGs are removed too, so that their primaries stop sending writes
Owner

а тут можно сделать как-то без перебора всех PG, в конце концов этот check на каждом изменении конфигурации пулов или PG гоняется. просто как-нибудь находить одну, если есть то выходить

а тут можно сделать как-то без перебора всех PG, в конце концов этот check на каждом изменении конфигурации пулов или PG гоняется. просто как-нибудь находить одну, если есть то выходить
@@ -476,0 +506,4 @@
void osd_t::rollback_removed_pools()
{
if (!bs)
Owner

вот тут хочется, чтобы этот rollback вообще не вызывался даже, если стор новый. можно хоть даже флаг какой-то добавить, типа if !bs || !bs->is_new() -> не вызывать.

причём делать это не даже не тут на rollback(), а прям на check(), если он будет основной точкой входа.

вот тут хочется, чтобы этот rollback вообще не вызывался даже, если стор новый. можно хоть даже флаг какой-то добавить, типа if !bs || !bs->is_new() -> не вызывать. причём делать это не даже не тут на rollback(), а прям на check(), если он будет основной точкой входа.
@@ -476,0 +515,4 @@
{
return;
}
for (auto & pp: pgs)
Owner

аналогично, перебирать конфигурации всех активных PG - это прям стрём, нужно как-то лёгенько по какому-нибудь std::set ID пулов (имеющих unstable версии, вероятно) проходиться, находить если любая 1 pg есть в pgs, то не трогать его пока, как-то так.

короче надо так, чтобы: при обычной работе оно вообще ничего не делало и проверка была O(1), делать начинало только а) при старте б) при изменении конфигурации пулов, когда из неё пропадает пул, а в остальное время не смотрело на изменения PG. ну и да как выше уже написал - чтоб это вообще было только для старого стора. а то стрёмно мне чтобы это не полезло роллбэкать что-то когда не надо, хочется максимально ограничить кейсы.

причём теоретически в момент когда пропадает пул, если уж мы пытаемся красиво обработать кейс, когда злобный админ грохает пул не удалив данные и не остановив клиентов (а я так понимаю это как раз кейс который ты хочешь обработать в этом ПР...), unstable-ов может быть больше, а когда уже реально пропадут PG и пулы и мы реально пойдём всё уже удалять, их будет меньше, т.е. список-то объектов перезапрашивать конечно

аналогично, перебирать конфигурации всех активных PG - это прям стрём, нужно как-то лёгенько по какому-нибудь std::set ID пулов (имеющих unstable версии, вероятно) проходиться, находить если любая 1 pg есть в pgs, то не трогать его пока, как-то так. короче надо так, чтобы: при обычной работе оно вообще ничего не делало и проверка была O(1), делать начинало только а) при старте б) при изменении конфигурации пулов, когда из неё пропадает пул, а в остальное время не смотрело на изменения PG. ну и да как выше уже написал - чтоб это вообще было только для старого стора. а то стрёмно мне чтобы это не полезло роллбэкать что-то когда не надо, хочется максимально ограничить кейсы. причём теоретически в момент когда пропадает пул, если уж мы пытаемся красиво обработать кейс, когда злобный админ грохает пул не удалив данные и не остановив клиентов (а я так понимаю это как раз кейс который ты хочешь обработать в этом ПР...), unstable-ов может быть больше, а когда уже реально пропадут PG и пулы и мы реально пойдём всё уже удалять, их будет меньше, т.е. список-то объектов перезапрашивать конечно
koolkhel force-pushed removed-pools-v1 from 5ddaaaefc9 to c216e4929f 2026-10-28 12:25:50 +03:00 Compare
Author
Contributor

Переделал, запушил в ту же ветку.

  • Одна точка входа, только для старого стора. check_removed_pools() вызывается при загрузке PG (старт или перезагрузка из etcd) и при изменении /config/pools. Первой строкой там if (!bs || bs->is_new()) return;, is_new() добавил в blockstore_i (true в новом сторе). Для нового стора не вызывается вообще ничего, в том числе get_unstable_objects().
  • В обычной работе ничего не делает. Есть std::set<pool_id_t> rm_pools: удалённые пулы, по которым ещё надо откатить. Обычно он пустой, и изменения pg/config и остановка PG проверяют только его: rm_pools.size() в хуке и rm_pools.find(pg.pool_id) в finish_stop_pg. Перебора всех PG больше нет.
  • Когда пул пропадает. При изменении /config/pools прохожу pool_config (пулы, не PG) и сравниваю с набором пулов, которые были в конфиге раньше. Тот, который был и пропал, попадает в rm_pools. На старте в rm_pools попадают несуществующие пулы из списка unstable-объектов стора: пул могли удалить, пока OSD лежал.
  • Ждём, пока реально пропадут PG. Для пула из rm_pools смотрю только его PG: pgs.lower_bound({ pool_id, 0 }) до первой не-OFFLINE и его pg_config до первой с config_exists. Если что-то есть, пул остаётся в rm_pools до следующего изменения pg/config или остановки его PG.
  • Список перезапрашивается. Стор листится только в момент отката, когда PG пула уже удалены из конфига и остановлены. При самом удалении пула не листится ничего, так что unstable-версии, которые к тому времени успели стабилизироваться, в откат не попадают.
  • pgs в хуке теперь вычисляется один раз.
  • Если пул с тем же ID создали заново до отката, он убирается из rm_pools без отката.

Про osd_flush.cpp:485: там был continue для существующих пулов, а не выход, то есть срабатывало для любого удалённого пула, у которого уже не было PG. Но этого кода теперь нет.

Тесты: osd_test (test_rollback_removed_pool_on_start, _online, и новый _idle: при обычных изменениях пулов и PG стор не листится, а для нового стора не листится вообще), test_blockstore_v1, OLD=1 tests/test_rm_pool_unstable.sh (все OSD пишут "Rolled back 8 …" в обеих фазах). Ветка перебазирована на текущий master.

Переделал, запушил в ту же ветку. - **Одна точка входа, только для старого стора.** `check_removed_pools()` вызывается при загрузке PG (старт или перезагрузка из etcd) и при изменении `/config/pools`. Первой строкой там `if (!bs || bs->is_new()) return;`, `is_new()` добавил в `blockstore_i` (true в новом сторе). Для нового стора не вызывается вообще ничего, в том числе `get_unstable_objects()`. - **В обычной работе ничего не делает.** Есть `std::set<pool_id_t> rm_pools`: удалённые пулы, по которым ещё надо откатить. Обычно он пустой, и изменения `pg/config` и остановка PG проверяют только его: `rm_pools.size()` в хуке и `rm_pools.find(pg.pool_id)` в `finish_stop_pg`. Перебора всех PG больше нет. - **Когда пул пропадает.** При изменении `/config/pools` прохожу `pool_config` (пулы, не PG) и сравниваю с набором пулов, которые были в конфиге раньше. Тот, который был и пропал, попадает в `rm_pools`. На старте в `rm_pools` попадают несуществующие пулы из списка unstable-объектов стора: пул могли удалить, пока OSD лежал. - **Ждём, пока реально пропадут PG.** Для пула из `rm_pools` смотрю только его PG: `pgs.lower_bound({ pool_id, 0 })` до первой не-OFFLINE и его `pg_config` до первой с `config_exists`. Если что-то есть, пул остаётся в `rm_pools` до следующего изменения `pg/config` или остановки его PG. - **Список перезапрашивается.** Стор листится только в момент отката, когда PG пула уже удалены из конфига и остановлены. При самом удалении пула не листится ничего, так что unstable-версии, которые к тому времени успели стабилизироваться, в откат не попадают. - `pgs` в хуке теперь вычисляется один раз. - Если пул с тем же ID создали заново до отката, он убирается из `rm_pools` без отката. Про `osd_flush.cpp:485`: там был `continue` для существующих пулов, а не выход, то есть срабатывало для любого удалённого пула, у которого уже не было PG. Но этого кода теперь нет. Тесты: `osd_test` (`test_rollback_removed_pool_on_start`, `_online`, и новый `_idle`: при обычных изменениях пулов и PG стор не листится, а для нового стора не листится вообще), `test_blockstore_v1`, `OLD=1 tests/test_rm_pool_unstable.sh` (все OSD пишут "Rolled back 8 …" в обеих фазах). Ветка перебазирована на текущий master.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u removed-pools-v1:koolkhel-removed-pools-v1
git checkout koolkhel-removed-pools-v1
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vitalif/vitastor#106
No description provided.