calypr · lbeckman314 · Dec 22, 2025 · Copilot · Dec 22, 2025 · Copilot
diff --git a/config/kubernetes/executor-job.yaml b/config/kubernetes/executor-job.yaml
@@ -1,3 +1,4 @@
+# TODO: Remove all K8s artifacts in favor of Helm configs (confusing to have both)
 # Task Executor
 apiVersion: batch/v1
 kind: Job

diff --git a/database/boltdb/events.go b/database/boltdb/events.go
@@ -173,6 +173,9 @@ func transitionTaskState(tx *bolt.Tx, id string, target tes.State) error {
 		return fmt.Errorf("Won't switch between two terminal states: %s -> %s",
 			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,
-	// e.g. Accessing invalid object shpuld be SYSTEM_ERROR every time,
+	// e.g. Accessing invalid object should be SYSTEM_ERROR every time,
-	// e.g. Accessing invalid object shpuld be SYSTEM_ERROR every time,
+	// e.g. Accessing invalid object should 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.
-	// 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.
-	// 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.
 	case tes.TerminalState(current) && !tes.TerminalState(target):
 		// Error when trying to switch out of a terminal state to a non-terminal one.
 		return fmt.Errorf("Unexpected transition from %s to %s", current.String(), target.String())

diff --git a/database/mongodb/events.go b/database/mongodb/events.go
@@ -37,6 +37,9 @@ func (db *MongoDB) WriteEvent(ctx context.Context, req *events.Event) error {
 
 	case events.Type_TASK_STATE:
 		retrier := util.NewRetrier()
+		// TODO: This could be where the invalid state transition errors are originating from (credit @Sai)
+		// Check how this interacts with K8s BackoffLimit on Job spec
+		// Ideally we would have users be able to control their own retry policies outside of internal Funnel's logic
 		retrier.ShouldRetry = func(err error) bool {
 			_, isTransitionError := err.(*tes.TransitionError)
 			return !isTransitionError && err != tes.ErrNotFound && err != tes.ErrNotPermitted

diff --git a/tes/states.go b/tes/states.go
@@ -21,6 +21,7 @@ type TransitionError struct {
 }
 
 func (te *TransitionError) Error() string {
+	// TODO: This error is being thrown after valid worker restarts (e.g. K8s)
 	return fmt.Sprintf("invalid state transition from %s to %s",
 		te.From.String(), te.To.String())
 }