Skip to content

fix: repair missing action rows on status update - #7812

Open
Rohithmatham12 wants to merge 1 commit into
flyteorg:mainfrom
Rohithmatham12:codex/flyte-self-heal-missing-action-status
Open

Rohithmatham12 wants to merge 1 commit into
flyteorg:mainfrom
Rohithmatham12:codex/flyte-self-heal-missing-action-status

Conversation

@Rohithmatham12

Copy link
Copy Markdown
Contributor

Summary

  • return a repository-level ErrActionNotFound when UpdateActionPhase matches no row and the action truly does not exist
  • map that sentinel to a NotFound status in InternalRunService.UpdateActionStatus
  • make the actions watcher re-record the TaskAction and retry UpdateActionStatus when the run service reports NotFound
  • preserve stale/backward phase no-op behavior by checking row existence before returning NotFound

Fixes #7257.

Testing

  • GOCACHE=/private/tmp/flyte-go-cache GOMODCACHE=/private/tmp/flyte-go-mod-cache go test ./actions/k8s -run 'TestNotifyRunService_RecordAndRetryWhenStatusUpdateReturnsNotFound$'\n- GOCACHE=/private/tmp/flyte-go-cache GOMODCACHE=/private/tmp/flyte-go-mod-cache go test ./runs/service -run 'TestUpdateActionStatus_MissingActionReturnsNotFoundStatus$'\n- GOCACHE=/private/tmp/flyte-go-cache GOMODCACHE=/private/tmp/flyte-go-mod-cache go test ./runs/repository/impl -run 'TestUpdateActionPhase_(MissingActionReturnsNotFound|BlocksBackwardFromSucceeded)$'

@github-actions github-actions Bot added the flyte2 label Aug 8, 2026
}
if rowsAffected > 0 {
r.notifyActionUpdate(ctx, actionID)
} else if _, err := r.GetAction(ctx, actionID); err != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any database error is reported as "not found". GetAction returns an error for timeouts and connection problems too. So those become CodeNotFound and trigger an unneeded re-record.

Comment thread actions/k8s/client.go Outdated

func (c *ActionsClient) recordActionInRunService(ctx context.Context, taskAction *executorv1.TaskAction, update *ActionUpdate, actionKey []byte) bool {
recordReq := buildRecordActionRequest(ctx, taskAction, update)
if _, err := c.runClient.RecordAction(ctx, connect.NewRequest(recordReq)); err != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RecordAction failures inside the response are treated as success. We should catch the resp here as well and then check resp.Msg.GetStatus().GetCode() to log failures.

@Rohithmatham12
Rohithmatham12 force-pushed the codex/flyte-self-heal-missing-action-status branch from f54ffef to 533edee Compare October 3, 2026 23:32
@Rohithmatham12

Copy link
Copy Markdown
Contributor Author

Rebased on current main and addressed the review feedback.

What changed:

  • Preserved the upstream in-band UpdateActionStatus rejection handling while keeping the missing-action retry path.
  • RecordAction now treats non-zero resp.Status.Code as a failed record, logs it, and does not add the action to the recorded filter.
  • UpdateActionPhase now only converts the fallback lookup to ErrActionNotFound when the action is truly missing; lookup/DB errors are returned as internal verification failures instead of being misreported as not found.
  • Added regression coverage for retrying RecordAction when the run service rejects the record in-band.

Validation:

  • git diff --check
  • GOCACHE=/private/tmp/flyte-go-cache GOMODCACHE=/private/tmp/flyte-go-mod-cache go test ./actions/k8s -run 'TestNotifyRunService_(RejectedStatusUpdateSkipsTerminalLabel|RecordAndRetryWhenStatusUpdateReturnsNotFound|RetriesRecordActionWhenRunServiceRejectsRecord)$'
  • GOCACHE=/private/tmp/flyte-go-cache GOMODCACHE=/private/tmp/flyte-go-mod-cache go test ./runs/service -run 'TestUpdateActionStatus_MissingActionReturnsNotFoundStatus$'

I also attempted the focused repository DB tests, but the local embedded Postgres test port was already occupied on this machine:

failed to start embedded postgres on port 15432: process already listening on port 15432

That package should get a clean DB process in CI.

Signed-off-by: Rohithmatham12 <rohithmatham@gmail.com>
@Rohithmatham12
Rohithmatham12 force-pushed the codex/flyte-self-heal-missing-action-status branch from 533edee to 3448964 Compare October 3, 2026 23:33

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InternalRunService: silent data loss when RecordAction fails before UpdateActionStatus

2 participants