Harden recall and capture reliability - #78
Conversation
Keeps recall fast and useful while making automatic capture bounded, nonblocking, and retry-safe.
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![vorflux[bot]](https://avatars.githubusercontent.com/in/2483280?s=60&v=4){kind=small}
| await captureSessionEnd(sessionID); | ||
| snapshots.delete(sessionID); | ||
| activeSessions.delete(sessionID); | ||
| if (await captureSessionEnd(sessionID)) { |
There was a problem hiding this comment.
P2 — Retry terminal capture before disposal completes. Retaining the failed batch in these in-memory maps only helps if another lifecycle event arrives. On server.instance.disposed, this is the final event: a transient failure or the new 3-second timeout makes captureSessionEnd() return false, this handler returns, and process exit discards the only retry state. The stable customId makes a bounded retry safe, so please retry pending terminal batches inside this disposal path (or persist them outside process memory).
There was a problem hiding this comment.
more of a fall open kind of thing, i think we should have this? what do you think @sreedharsreeram
There was a problem hiding this comment.
I agree the hook should fail open. My concern is narrower: retaining the batch in these maps only enables retry if another lifecycle event arrives, but server.instance.disposed is the terminal event and process exit discards the maps. A bounded retry within the existing shutdown deadline would still fail open while covering one transient timeout; after that it can return and allow shutdown. If accepting possible loss of the final batch is intentional, the current behavior is consistent with that tradeoff.
Merge activity
|
2 participants
Keeps recall fast and useful while making automatic capture bounded, nonblocking, and retry-safe.