Conversation
✅ Deploy Preview for funnel-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR adds TODO comments to investigate invalid state transition errors that occur after valid worker restarts in Kubernetes environments. The changes are purely documentation-focused, adding context for future investigation without modifying any code logic.
- Added TODO comments identifying potential sources of invalid state transition errors during K8s worker restarts
- Documented concerns about retry policy interactions with K8s BackoffLimit settings
- Added note about removing K8s artifacts in favor of Helm configurations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tes/states.go | Added TODO noting that the TransitionError is thrown after valid worker restarts |
| database/mongodb/events.go | Added TODO suggesting this retrier logic may be the source of invalid state transition errors and noting interaction with K8s BackoffLimit |
| database/boltdb/events.go | Added comment explaining executor restart behavior and terminal state transitions |
| config/kubernetes/executor-job.yaml | Added TODO to remove K8s artifacts in favor of Helm configs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // In case where restarts of executor are allowed, this line should not be encountered | ||
| // e.g. Accessing invalid object shpuld be SYSTEM_ERROR every time, | ||
| // but if a retry mechanism would work (e.g. unavailable for a few seconds), then Task Worker should allow Executor to restart as expected. |
There was a problem hiding this comment.
This comment is difficult to parse and unclear. Consider breaking it into multiple sentences or restructuring for clarity. The relationship between "restarts of executor", "invalid object", "SYSTEM_ERROR", and "retry mechanism" is confusing as currently written.
| // In case where restarts of executor are allowed, this line should not be encountered | |
| // e.g. Accessing invalid object shpuld be SYSTEM_ERROR every time, | |
| // but if a retry mechanism would work (e.g. unavailable for a few seconds), then Task Worker should allow Executor to restart as expected. | |
| // When executor restarts are allowed, this branch should not be reached. | |
| // For example, repeatedly accessing an invalid object should always result in SYSTEM_ERROR (a terminal state) with no retries. | |
| // In contrast, transient errors that can be retried (e.g. a service being unavailable for a few seconds) should be handled before the task reaches a terminal state, allowing the executor to restart as expected. |
| current, target) | ||
|
|
||
| // In case where restarts of executor are allowed, this line should not be encountered | ||
| // e.g. Accessing invalid object shpuld be SYSTEM_ERROR every time, |
There was a problem hiding this comment.
Spelling error: "shpuld" should be "should"
| // e.g. Accessing invalid object shpuld be SYSTEM_ERROR every time, | |
| // e.g. Accessing invalid object should be SYSTEM_ERROR every time, |
fix/invalid-state-transition -> develop
fix/invalid-state-transition -> developfix/invalid-state-transition → develop
Overview 🌀
This PR resolves the Invalid State Transition error when resubmitting tasks via an external retry mechanism (e.g. K8s BackoffLimit).
Current Behavior⚠️
s3-invalid.json{ "name": "S3 Storage example (invalid)", "description": "Task inputs and outputs can be Cloud Storage URLs (Invalid Test)", "executors": [ { "image": "ubuntu", "command": ["md5sum", "/tmp/README.md"] } ], "inputs": [ { "name": "input", "description": "Download a file from S3 Storage", "url": "s3://funnel-testing-east/ERROR", <----- Non-existent object set here "path": "/tmp/README.md" } ] }Note
Tested with Helm Charts 0.1.71 (2025-12-14) and 0.1.75 (2025-12-22):