fix(replybot): clear stale error/wait/retries on state transitions#136
Open
nandanrao wants to merge 1 commit into
Open
fix(replybot): clear stale error/wait/retries on state transitions#136nandanrao wants to merge 1 commit into
nandanrao wants to merge 1 commit into
Conversation
Transient fields (error, wait, waitStart, retries) were leaking into states where they no longer apply. For example, a participant in WAIT_EXTERNAL_EVENT that received a MACHINE_REPORT error would keep the stale wait object in the new ERROR state. Similarly, END and QOUT transitions retained error/retry context from prior states. This is visible to users via the Monitor tab's raw state_json viewer and pollutes the error_tag/fb_error_code computed columns used by StatesList filtering. Changes to apply(): - RESPOND: also clear wait/waitStart (already cleared error/retries) - RESPOND_AGAIN (REDO): clear error/wait, keep retries for Dean backoff - WAIT_RESPONSE (QOUT): clear error/retries - HANDOFF: clear error/retries - WAIT_EXTERNAL_EVENT: clear error/retries - END: clear error/wait/retries - BLOCKED: clear wait/waitStart (keep error/retries) - ERROR: clear wait/waitStart (keep error/retries) Invariant enforced: transient fields exist only in their owning states - error: ERROR, BLOCKED - wait/waitStart: WAIT_EXTERNAL_EVENT - retries: RESPONDING, ERROR, BLOCKED Added 6 logs-based tests covering real reachable transitions. Updated 1 existing test that was asserting on the stale waitStart leak.
✅ Deploy Preview for vlab-research canceled.
|
✅ Deploy Preview for virtuallab-videos canceled.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Transient fields (
error,wait,waitStart,retries) were leaking into states where they no longer apply. For example:WAIT_EXTERNAL_EVENTthat received aMACHINE_REPORTerror would keep the stalewaitobject in the newERRORstateENDandQOUTtransitions retained error/retry context from prior statesRESPOND_AGAIN(REDO) retained staleerrorwhen Dean retried a failed participantThis is visible to users via the Monitor tab's raw
state_jsonviewer and pollutes theerror_tag/fb_error_codecomputed columns used by StatesList filtering.Solution
Enforce an invariant in
apply(): transient fields exist only in their owning states:error→ERROR,BLOCKEDwait/waitStart→WAIT_EXTERNAL_EVENTretries→RESPONDING,ERROR,BLOCKED(Dean retry-eligible states)Changes to
apply()(8 cases)RESPONDwait,waitStart(error+retries already cleared)RESPOND_AGAIN(REDO)error,waitretries— Dean's max-attempts backoff guardWAIT_RESPONSE(QOUT)error,retriesHANDOFFerror,retrieswait(being set)WAIT_EXTERNAL_EVENTerror,retrieswait(being set)ENDerror,wait,retriesBLOCKEDwait,waitStarterror(being set),retriesERRORwait,waitStarterror(being set),retriesDean safety
Dean's queries always pair
current_statewith the tag/code filter (e.g.current_state = 'ERROR' AND error_tag = ANY(...)), so it already self-gates. Theretriesfield is preserved inRESPOND_AGAINso Dean's max-attempts guard (JSON_ARRAY_LENGTH(state_json->'retries') < N) continues to work.Testing
getState(log)— each exercises a real reachable transitionwaitStartleak (5 from a prior wait). Renamed and updated to assert freshwaitStart(10 from the new echo).Rollout plan
state_json.error/wait/retriesfrom existing rows. This also recomputes the stored computed columns (error_tag,fb_error_code,timeout_date,next_retry). To be done after confirming A is stable in production.