Skip to content

Commit 847a92b

Browse files
committed
fix: Reject checkpoints from resolvers of an ended sync
A Scheduler reused across syncs kept resolvers of a cancelled sync running. Their flushes went onto the next sync's stream, so the CLI could persist cursors past the rows it committed.
1 parent 1b4fde9 commit 847a92b

2 files changed

Lines changed: 59 additions & 5 deletions

File tree

‎cloudquery/sdk/internal/checkpoint.py‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ class Scope:
3030
spawned is finished.
3131
"""
3232

33-
def __init__(self, parent: Optional[Scope]):
33+
def __init__(self, parent: Optional[Scope], sync: Optional[object]):
3434
self.parent = parent
35+
# the sync the scope belongs to, so that resolvers still running after it ended cannot checkpoint into the next one
36+
self.sync = sync
3537
# child scopes that are not finished yet
3638
self.open_children: Set[Scope] = set()
3739
self.resolving = True
@@ -45,9 +47,10 @@ def finished(self) -> bool:
4547
class _Pending:
4648
"""A checkpoint waiting for scopes to finish and for earlier checkpoints of its keys to be sent."""
4749

48-
def __init__(self, msg: SyncCheckpointMessage, keys: List[str]):
50+
def __init__(self, msg: SyncCheckpointMessage, keys: List[str], sync: object):
4951
self.msg = msg
5052
self.keys = keys
53+
self.sync = sync
5154
self.blockers = 0
5255
self.dependents: List[_Pending] = []
5356

@@ -68,22 +71,25 @@ def __init__(self):
6871
self._res: Optional[queue.Queue] = None
6972
# newest unsent checkpoint of each key
7073
self._unsent: Dict[str, _Pending] = {}
74+
self._sync: Optional[object] = None
7175

7276
def start(self, res: queue.Queue) -> None:
7377
"""Sends later checkpoints on res. Called by the scheduler when a sync starts."""
7478
with self._lock:
7579
self._res = res
80+
self._sync = object()
7681
self._unsent = {}
7782

7883
def stop(self) -> None:
7984
"""Drops unsent checkpoints and rejects later ones. Called by the scheduler when a sync ends."""
8085
with self._lock:
8186
self._res = None
87+
self._sync = None
8288
self._unsent = {}
8389

8490
def new_scope(self, parent: Optional[Scope]) -> Scope:
8591
"""Returns a scope for a table resolver call, before the call is submitted."""
86-
scope = Scope(parent)
92+
scope = Scope(parent, parent.sync if parent is not None else self._sync)
8793
if parent is not None:
8894
with self._lock:
8995
parent.open_children.add(scope)
@@ -145,6 +151,11 @@ def schedule(
145151
"state checkpoints can only be flushed while the scheduler syncs, "
146152
"e.g. from a table resolver or the post-sync hook"
147153
)
154+
for scopes, _, _ in checkpoints:
155+
if any(s is not None and s.sync is not self._sync for s in scopes):
156+
raise RuntimeError(
157+
"state checkpoints of a table resolver can only be flushed during its own sync"
158+
)
148159
for scopes, keys, msg in checkpoints:
149160
self._schedule(scopes, keys, msg)
150161

@@ -154,7 +165,7 @@ def _schedule(
154165
keys: List[str],
155166
msg: SyncCheckpointMessage,
156167
) -> None:
157-
pending = _Pending(msg, keys)
168+
pending = _Pending(msg, keys, self._sync)
158169
for scope in {s for s in scopes if s is not None}:
159170
# the scope's own resources are on the queue already: resolvers put each
160171
# resource on it before they resume, so only its relations can be in flight
@@ -176,7 +187,7 @@ def _send(self, ready: List[_Pending]) -> None:
176187
"""Puts ready checkpoints on the queue, then the ones that only waited for them. Called with the lock held."""
177188
while ready:
178189
pending = ready.pop()
179-
if self._res is not None:
190+
if pending.sync is self._sync:
180191
self._res.put(pending.msg)
181192
for key in pending.keys:
182193
if self._unsent.get(key) is pending:

‎tests/scheduler/checkpoints.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,3 +352,46 @@ def resolve(self, client, parent_resource) -> Generator[Any, None, None]:
352352
finally:
353353
scheduler.shutdown()
354354
assert scopes[0] is not None
355+
356+
357+
class CursorResolver(TableResolver):
358+
def __init__(self, state: StateClient, tag: str):
359+
super().__init__(Table("items", [Column("id", pa.int64())]))
360+
self._state = state
361+
self._tag = tag
362+
363+
def resolve(self, client, parent_resource) -> Generator[Any, None, None]:
364+
for item_id in range(1, 30):
365+
time.sleep(0.02)
366+
yield {"id": item_id}
367+
self._state.set_key("cursor", f"{self._tag}-{item_id}")
368+
self._state.flush()
369+
370+
371+
def test_resolvers_of_an_ended_sync_do_not_checkpoint_into_the_next_one():
372+
scheduler = Scheduler(4)
373+
try:
374+
ended = scheduler.sync(
375+
TestClient(),
376+
[
377+
CursorResolver(
378+
MemoryStateClient(checkpoints=scheduler.checkpoints), "ended"
379+
)
380+
],
381+
)
382+
for _ in range(5):
383+
next(ended)
384+
ended.close()
385+
386+
state = MemoryStateClient(checkpoints=scheduler.checkpoints)
387+
scheduler.set_post_sync_hook(state.flush)
388+
messages = list(scheduler.sync(TestClient(), [CursorResolver(state, "next")]))
389+
finally:
390+
scheduler.shutdown()
391+
cursors = [
392+
checkpoint_rows(m)["cursor"]
393+
for m in messages
394+
if isinstance(m, SyncCheckpointMessage)
395+
]
396+
assert cursors
397+
assert all(cursor.startswith("next-") for cursor in cursors)

0 commit comments

Comments
 (0)