Repository navigation
Conversation
A resolver still running after its sync ended could change the state client of the next sync, e.g. one a plugin keeps on an attribute. Its key then failed every flush of that sync, as a failed flush keeps its changes, and the next sync read its value. set_key now raises before it records the change.
| """ | ||
| with self._flush_lock: | ||
| with self._lock: | ||
| for key in list(self.changes): |
There was a problem hiding this comment.
A state client that is kept across syncs is still blocked. If a change is set during its own sync and that sync is closed before a flush, every later flush raises, and get_key returns the stale value. This happens on main too. Now flush and get_key drop these changes and restore the last flushed value. A new test covers it.
| return self.mem.get(key) | ||
|
|
||
| def set_key(self, key: str, value: str) -> None: | ||
| scope = self.checkpoints.current_scope() if self.checkpoints else None |
There was a problem hiding this comment.
The check and the write are not atomic, so the sync can switch between them. Because flush now drops stale changes, this is harmless.
|
|
||
| def set_key(self, key: str, value: str) -> None: | ||
| scope = self.checkpoints.current_scope() if self.checkpoints else None | ||
| if self.checkpoints and self.checkpoints.superseded(scope): |
There was a problem hiding this comment.
A leftover resolver that calls set_key on its own old client now raises, and on main it doesn't. So the "aren't affected" claim in the description is not true. Now set_key ignores these changes, the same as _send does with checkpoints of ended syncs.
| """Returns the scope of the table resolver running in this context, or None outside table resolvers.""" | ||
| return self._scope.get() | ||
|
|
||
| def superseded(self, scope: Optional[Scope]) -> bool: |
There was a problem hiding this comment.
The same check was in check_scope and schedule. I replaced both with one superseded(scope).
|
|
||
|
|
||
| def test_resolvers_of_an_ended_sync_cannot_change_the_state_of_the_next_one(): | ||
| plugin_attributes: Dict[str, StateClient] = {} |
There was a problem hiding this comment.
Removed the comment (shared → plugin_attributes), the waits are now asserted, and the test now expects no error.
🤖 I have created a release *beep* *boop* --- ## [0.1.56](v0.1.55...v0.1.56) (2026-10-20) ### Bug Fixes * Reject state changes from resolvers of an ended sync ([#417](#417)) ([bbd5b2e](bbd5b2e)) * Send checkpoints from resource hooks after their resources ([#419](#419)) ([fe8cc8f](fe8cc8f)) * Use the computed queue size for child resolver pools ([#416](#416)) ([0a4b408](0a4b408)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
2 participants
Summary
Follow-up to #415. Its rejection of checkpoints from an ended sync's resolvers can leave the next sync unable to save any state.
When a plugin keeps the current sync's state client on an attribute its resolvers read, a resolver left over from an ended sync can call
set_keyon the next sync's client. That key is tied to the ended sync's scope, so the nextflush()is rejected. A failed flush keeps its changes for a retry, so the key stays and every later flush of the new sync raises too, including the post-sync hook. The new sync can also read the leftover's value back throughget_key. A state client reused across syncs hits the same problem when a change set during one sync is not flushed before that sync closes, which happens on main too.Once another sync runs, state changes made by resolvers of an ended sync are now dropped, as
_sendalready does with their checkpoints.set_keyignores them, andflushandget_keydiscard any that are already recorded, restoring the key's last flushed value.New tests cover a leftover resolver writing into the next sync's client and a reused client whose sync closed before a flush.