test: fix flaky test_incremental_repair_race_window_promotes_unrepaired_data
The test waited for two "Finished tablet repair" log messages on the
coordinator, expecting one per tablet. But there are two log sources
that emit messages matching this pattern:
repair module (repair/repair.cc:2329):
"Finished tablet repair for table=..."
topology coordinator (topology_coordinator.cc:2083):
"Finished tablet repair host=..."
When the coordinator is also a repair replica (always the case with
RF=3 and 3 nodes), both messages appear in the coordinator log for the
same tablet within 1ms of each other. The test consumed both, thinking
both tablets were done, while the second tablet repair was still running.
From the CI failure logs:
04:08:09.658 Found: repair[...]: Finished tablet repair for table=...
global_tablet_id=e42fd650-3542-11f1-9756-85403784a622:0
04:08:09.660 Found: raft_topology - Finished tablet repair host=...
tablet=e42fd650-3542-11f1-9756-85403784a622:0
Both messages are for tablet :0. Tablet :1 repair had not finished yet.
The test then wrote keys 20-29 while the second tablet repair was still
in progress. That repair flushed the memtable (via
prepare_sstables_for_incremental_repair), including keys 20-29 in the
repair scan, and mark_sstable_as_repaired set repaired_at=2 on the
resulting sstable. This caused the assertion failure on servers[0]:
"should not have post-repair keys in repaired sstables, got:
{20, 21, 22, 23, 24, 25, 26, 27, 28, 29}"
Fix by matching "Finished tablet repair host=" which is unique to the
topology coordinator message and avoids the ambiguity.
Also fix an incorrect comment that said being_repaired=null when at that
point in the test being_repaired is still set to the session_id (the
delay_end_repair_update injection prevents end_repair from running).
Fixes: SCYLLADB-1478
Closes scylladb/scylladb#29444
This commit is contained in:
@@ -911,18 +911,27 @@ async def test_incremental_repair_race_window_promotes_unrepaired_data(manager:
|
||||
)
|
||||
task_id = repair_response['tablet_task_id']
|
||||
|
||||
# "Finished tablet repair" is logged once per tablet after mark_sstable_as_repaired()
|
||||
# has completed on all replicas for that tablet. With tablets=2 the coordinator logs
|
||||
# this message twice (once per tablet). We must wait for BOTH before writing
|
||||
# post-repair keys; waiting for only the first leaves the second tablet's repair in
|
||||
# progress, which can flush the memtable and mark newly-flushed sstables as repaired,
|
||||
# contaminating servers[0] and servers[2] with post-repair data in repaired sstables.
|
||||
# The topology coordinator logs "Finished tablet repair host=..." once per tablet
|
||||
# after mark_sstable_as_repaired() has completed on all replicas for that tablet.
|
||||
# With tablets=2, the coordinator logs this message twice (once per tablet).
|
||||
# We must wait for BOTH before writing post-repair keys; waiting for only the
|
||||
# first leaves the second tablet's repair in progress, which can flush the
|
||||
# memtable and mark newly-flushed sstables as repaired, contaminating servers[0]
|
||||
# and servers[2] with post-repair data in repaired sstables.
|
||||
#
|
||||
# IMPORTANT: We match "Finished tablet repair host=" specifically to avoid
|
||||
# matching the repair module's "Finished tablet repair for table=..." message
|
||||
# (repair/repair.cc), which is also logged on this node when the coordinator
|
||||
# happens to be a repair replica. Both messages appear for the same tablet,
|
||||
# so the generic "Finished tablet repair" pattern would consume two messages
|
||||
# for one tablet and miss the second tablet entirely.
|
||||
#
|
||||
# After both tablets complete, S1 is fully rewritten as S1'(repaired_at=2,
|
||||
# being_repaired=null) on every replica, but sstables_repaired_at in system.tablets is
|
||||
# being_repaired=session_id) on every replica, but sstables_repaired_at in system.tablets is
|
||||
# still 1, so is_repaired(1, S1'{repaired_at=2}) == false and S1' lands in the
|
||||
# UNREPAIRED compaction view on every replica. The race window is now open.
|
||||
pos, _ = await coord_log.wait_for("Finished tablet repair", from_mark=coord_mark)
|
||||
await coord_log.wait_for("Finished tablet repair", from_mark=pos)
|
||||
pos, _ = await coord_log.wait_for("Finished tablet repair host=", from_mark=coord_mark)
|
||||
await coord_log.wait_for("Finished tablet repair host=", from_mark=pos)
|
||||
|
||||
# --- Race window is open ---
|
||||
# Write post-repair keys 20-29. All nodes receive the writes into their memtables
|
||||
|
||||
Reference in New Issue
Block a user