Skip to content

Commit e5c29da

Browse files
committed
main: cap per-group chunk reads per poll cycle (round-robin fairness)
The lagging-first sort (#46) fixed one starvation and created another. Sorted-but-uncapped iteration let the most-lagging group's chunk loop run all the way to head before the next group got a read — and when that group's flushes kept failing (portola OCC conflicts), its position kept resetting and the loop relitigated the same range indefinitely. Observed in prod: team-2's cursor frozen at 16967279 for 1.5h across three pod restarts, applied_inserts=0, while team-50689 monopolized every poll cycle. Cap each group at _MAX_CHUNKS_PER_GROUP_PER_CYCLE (4) chunks per poll cycle. Sorted order still gives the laggiest group the first turn each cycle; the cap guarantees every other group also gets a turn in the same cycle. 4 chunks x cdc_chunk_snapshots gives each group a per-cycle read budget of ~7x the source arrival rate at current prod settings, so lag still drains while fairness holds. New test seeds one group 10k snapshots behind and one 100 behind, and asserts the healthy group reads within the SAME cycle while the lagging group reads exactly the cap. Verified to fail against the uncapped loop.
1 parent 8160c30 commit e5c29da

2 files changed

Lines changed: 84 additions & 9 deletions

File tree

tests/unit/test_main.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,62 @@ def fake_read(src_table, *, after_snapshot, end_snapshot, filter_expr=None):
10301030
)
10311031

10321032

1033+
def test_poll_cycle_lagging_group_cannot_monopolize_reads():
1034+
"""The per-cycle chunk cap turns lagging-first priority into round-robin:
1035+
a deeply-lagging group reads at most _MAX_CHUNKS_PER_GROUP_PER_CYCLE
1036+
chunks per cycle, then the healthy group gets its turn. Without the cap
1037+
(the first sort-only fix), the lagging group's chunk loop ran to head —
1038+
or forever when its flushes kept failing and its position kept resetting
1039+
— and the healthy destination's cursor froze (observed on portola:
1040+
team-2 starved for 1.5h while team-50689 relitigated the same range).
1041+
"""
1042+
from viaduck.main import _MAX_CHUNKS_PER_GROUP_PER_CYCLE
1043+
1044+
# dest-lagging is 10,000 snapshots behind; dest-healthy is 100 behind.
1045+
# Uncapped, dest-lagging's group would issue 100 chunk reads before
1046+
# dest-healthy saw a single one.
1047+
delivery = _make_delivery({"dest-lagging": 0, "dest-healthy": 9_900})
1048+
router = MagicMock()
1049+
cfg = _make_cfg([("dest-lagging", "lag"), ("dest-healthy", "ok")])
1050+
cfg.poll.cdc_chunk_snapshots = 100
1051+
1052+
read_calls: list[tuple[int, int]] = []
1053+
1054+
def fake_read(src_table, *, after_snapshot, end_snapshot, filter_expr=None):
1055+
read_calls.append((after_snapshot, end_snapshot))
1056+
return pa.table({"company": pa.array([], type=pa.string())})
1057+
1058+
router.build_filter_expr.return_value = None
1059+
1060+
with (
1061+
patch("viaduck.main.source.current_snapshot_id", return_value=10_000),
1062+
patch("viaduck.main.source.read_cdc", side_effect=fake_read),
1063+
):
1064+
_poll_cycle(
1065+
MagicMock(),
1066+
delivery,
1067+
MagicMock(),
1068+
router,
1069+
cfg,
1070+
["dest-lagging", "dest-healthy"],
1071+
{"lag": "dest-lagging", "ok": "dest-healthy"},
1072+
key_columns=[],
1073+
mode="append_only",
1074+
)
1075+
1076+
# The lagging group read exactly the cap, no more.
1077+
lagging_reads = [c for c in read_calls if c[0] < 9_900]
1078+
assert len(lagging_reads) == _MAX_CHUNKS_PER_GROUP_PER_CYCLE, (
1079+
f"lagging group should read exactly the per-cycle cap, got {len(lagging_reads)}: {lagging_reads}"
1080+
)
1081+
# The healthy group STILL got its read this cycle — the load-bearing
1082+
# assertion. Under the uncapped loop this list is empty.
1083+
healthy_reads = [c for c in read_calls if c[0] >= 9_900]
1084+
assert healthy_reads == [(9_900, 10_000)], f"healthy group must read within the same cycle, got {healthy_reads}"
1085+
# Order: lagging first (priority preserved), healthy after.
1086+
assert read_calls[0][0] == 0 and read_calls[-1][0] == 9_900
1087+
1088+
10331089
def test_poll_cycle_snapshot_at_zero():
10341090
"""Source snapshot 0 with positions at 0: nothing to read, triggers run."""
10351091
delivery = _make_delivery({"dest-1": 0})

viaduck/main.py

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,15 @@
6969

7070
log = logging.getLogger(__name__)
7171

72+
# Per-cycle read budget per cursor group. Bounds how many CDC chunks one
73+
# group may read before the poll cycle moves to the next group — the fairness
74+
# valve that keeps a deeply-lagging (or flush-failing, position-resetting)
75+
# destination from monopolizing the poll thread while healthy peers starve.
76+
# 4 chunks × cdc_chunk_snapshots=50 = 200 snapshots per group per cycle,
77+
# ~7× the source's per-cycle arrival rate — groups drain lag while every
78+
# other group still gets a turn each cycle.
79+
_MAX_CHUNKS_PER_GROUP_PER_CYCLE = 4
80+
7281

7382
def _start_progress_heartbeat(
7483
label: str,
@@ -806,14 +815,22 @@ def _poll_cycle(src_table, delivery, dest_pool, router, cfg, assigned_ids, rv_to
806815
epochs = {d: epoch for d, (_pos, epoch) in plan.items()}
807816
groups = _group_by_cursor(positions, assigned_ids)
808817

809-
# Iterate lowest cursor first: the most-lagging group reads before
810-
# a caught-up peer's turn can trip the buffer watermark. Under the
811-
# prior insertion-order iteration, a destination whose cursor was
812-
# further behind was silently starved whenever the first-iterated
813-
# group filled the buffer mid-chunk — the outer loop moved to the
814-
# next group and immediately hit `should_pause_reads()`, breaking
815-
# out without reading anything. Sorting flips the priority so the
816-
# laggiest destination gets first shot at each poll's read budget.
818+
# Iterate lowest cursor first — the most-lagging group gets the
819+
# first turn each cycle — but cap the chunks each group may read
820+
# per cycle so a deeply-lagging group cannot monopolize the poll
821+
# thread. Both halves are load-bearing:
822+
# - Insertion-order iteration (pre-sort) starved a lagging
823+
# destination whose config index was after a caught-up peer:
824+
# the peer's read filled the buffer and the lagging group's
825+
# first watermark check bounced it out with zero reads.
826+
# - Sorted-but-uncapped iteration (the first fix) starved the
827+
# HEALTHY destination instead: the lagging group's chunk loop
828+
# ran to head — or forever, when its flushes kept failing and
829+
# the position kept resetting — so the caught-up group never
830+
# got a read. Observed on portola: team-2's cursor frozen for
831+
# 1.5h while team-50689 relitigated the same range.
832+
# The cap turns strict priority into a round-robin with a
833+
# lagging-first bias: every group makes progress every cycle.
817834
for start_snap, dest_ids in sorted(groups.items()):
818835
if start_snap >= current_id:
819836
continue # already read through the current snapshot
@@ -828,7 +845,9 @@ def _poll_cycle(src_table, delivery, dest_pool, router, cfg, assigned_ids, rv_to
828845
chunk_size = cfg.poll.cdc_chunk_snapshots
829846
chunk_start = start_snap
830847
routing_error = False
831-
while chunk_start < current_id:
848+
chunks_this_group = 0
849+
while chunk_start < current_id and chunks_this_group < _MAX_CHUNKS_PER_GROUP_PER_CYCLE:
850+
chunks_this_group += 1
832851
if delivery.should_pause_reads():
833852
_log_watermark_paused("mid-chunk")
834853
break

0 commit comments

Comments
 (0)