Repository navigation
feat(realtime): add ordered fetch worker - #84
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c120c57c77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self._task is None: | ||
| self._task = asyncio.create_task(self._run()) | ||
| await self._queue.put(job) |
There was a problem hiding this comment.
When _deliver raises, or _run exits for any other exception, _task remains a completed non-None task, so subsequent calls never start another consumer. Up to queue_limit jobs then appear to enqueue successfully, while the next enqueue—or close() while the queue is full—blocks forever because nothing drains the queue; detect task.done() and surface the worker failure before accepting more jobs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9c0e30b. Enqueue now surfaces a completed worker task failure before accepting another job; the regression verifies the original delivery exception is returned.
| self._closed = True | ||
| if task is not None: | ||
| await self._queue.put(_STOP_WORKER) |
There was a problem hiding this comment.
If close() is cancelled while the bounded queue is full, cancellation can occur in await self._queue.put(...) after _closed has already become true but before the stop marker is inserted. The worker then drains the accepted jobs and waits indefinitely, and every later close() skips insertion because _closed is already true and consequently waits forever on that worker task.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9c0e30b. Stop-marker insertion now runs in a retained task shielded from caller cancellation, so a later close resumes the same shutdown; the regression cancels a full-queue close and verifies all accepted work drains.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0e30bab2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._raise_worker_failure() | ||
| if self._task is None: | ||
| self._task = asyncio.create_task(self._run()) | ||
| await self._queue.put(job) |
There was a problem hiding this comment.
When the queue is full and an enqueue is already suspended here, _deliver can raise after the pre-put _raise_worker_failure() check has run. The consumer then exits without freeing the queued slot, so this await never completes and continues holding _state_lock; close() and later enqueues consequently block forever as well. The new pre-put check is fresh evidence because it cannot observe a failure that occurs during backpressure; wait for both the queue insertion and worker task, and propagate the worker failure if it finishes first.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ca2cf88. A blocked enqueue now races its queue insertion against the worker task, cancels the pending put when the worker exits, and propagates the worker failure. The regression reproduces the full-queue delivery-failure race.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Scope
Internal worker only. Public channel configuration, session invalidation, and subscription lifecycle integration remain separate slices.
Verification