ateapi: backfill unset worker state during pod sync - #599
Open
Mesut Oezdil (mesutoezdil) wants to merge 3 commits into
Open
ateapi: backfill unset worker state during pod sync#599Mesut Oezdil (mesutoezdil) wants to merge 3 commits into
Mesut Oezdil (mesutoezdil) wants to merge 3 commits into
Conversation
The scheduler now skips any worker whose state is not STATE_ACTIVE, but the syncer only sets State when it creates a worker record. The update path diffs Ip, SandboxClass and Labels and never touches State, and the Redis store outlives ate-api-server while worker pods are not recreated on upgrade. Every worker record written before the State field existed therefore stays at STATE_UNSPECIFIED and is permanently unschedulable, so actor placement fails with ErrNoCapacity until each worker pod is deleted and recreated. Backfill the unset state to STATE_ACTIVE when syncing a running pod, which converges legacy records on the first startup sync. Only the unset state is backfilled, so a DRAINING worker whose pod name is reused is not resurrected. ValidateWorker already documents STATE_UNSPECIFIED as tolerated for backward compatibility, and workflow_resume rejects only DRAINING, so the scheduler's strict check stays as is.
| changed = true | ||
| } | ||
| if w.State == ateapipb.Worker_STATE_UNSPECIFIED { | ||
| // A worker record written before the State field existed. The store |
Collaborator
There was a problem hiding this comment.
Backwards incompatible changes at this stage of the project is ok. Also, I don't think it's safe to always set UNSPECIFIED to _ACTIVE, we can have a bug somewhere that unset the state field by mistake.
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
2 participants
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.
Follow-up to #517.
#517 added a scheduler filter that skips any worker whose state is not
STATE_ACTIVE:substrate/cmd/ateapi/internal/scheduling/scheduling.go
Lines 113 to 115 in 87698ea
But the syncer writes
Stateonly when it creates a worker record. The update path insyncWorkerToStorediffsIp,SandboxClassandLabelsand never touchesState, and no other code path writesStateonto an existing record.The store is Redis backed, so worker records outlive an ate-api-server rollout, and worker pods are not recreated by that rollout either. Every worker record written before the
Statefield existed therefore stays atSTATE_UNSPECIFIEDand is skipped by the scheduler for the whole life of its pod. On an existing cluster upgrading past #517, actor placement fails withErrNoCapacityuntil each worker pod is deleted and recreated. The startup sync does not help, because the field diff computeschanged == falseand never writes.