fix(controller): sort deprecatedVersions so an unchanged status is not rewritten every reconcile - #38
Merged
Conversation
Status().Update is skipped only when the mapped status DeepEquals the persisted one. deprecatedVersions was built from a map range, so a TWD with several drained versions mapped to a differently ordered slice on every reconcile, rewrote its status, and re-triggered itself through the watch: ~40 status PUT/s on the worst tenants, ~1,000/s fleet-wide. (cherry picked from commit 7e13417) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fails against the unsorted mapper on the first pass with a permutation of the same build IDs and passes with the sort, so the DeepEqual status-write skip cannot regress into a reconcile loop again. Refs SHA-1469. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Cherry-pick of upstream temporalio#458 (7e13417, first shipped in v1.8.1) onto
v1.6.0-atlan, plus a regression test.status.deprecatedVersionsis now sorted by build ID before it is assigned, so a TWD whose state has not changed maps to an identical status on every reconcile. Refs SHA-1469.Why it loops
Reconcileskips itsStatus().Updateonly when the mapped statusDeepEquals the persisted one (worker_controller.go).deprecatedVersionswas built from arangeover theDeploymentsmap, so its order changed on every reconcile. For any TWD with several deprecated versions the guard never matched: every reconcile issued aPUT .../status, the write fired a watch event on the TWD, and the controller reconciled it again immediately. The 10sRequeueAfternever gets a chance to matter.Evidence (2026-09-08)
On the worst tenant, 3 of 80 TWDs with 12-13 drained versions each produced ~330 of ~405 status writes per 20s (~5.5/s per CR),
controller_runtime_reconcile_totalwas 100%requeue_afterat 44/s, and two consecutive GETs of such a CR differed only in the order ofdeprecatedVersions. Reorder probability grows with the number of entries andsunset.deleteDelayis 24h, so the loop switches on after a deploy burst and off a day later. This is what trippedHighApiServerRequestCounton those tenants.The change
state_mapper.go:slices.SortStableFuncbyBuildIDimmediately beforestatus.DeprecatedVersions = deprecatedVersions. Byte-identical to upstream apart from the merged import block; the upstream comment is kept verbatim so future rebases see the same text. Build ID is always set and unique per entry, which is why it is the sort key rather thandrainedSince.Test
TestMapToStatusDeprecatedVersionsOrderIsStablemaps one current plus twelve drained versions, inserted in descending order, twenty times and asserts ascending build-ID order. Against the unsorted mapper it fails on the first pass with a permutation of the same IDs; with the sort it passes.Verified locally on this branch:
go vet,gofmt,gci,golangci-lint(0 new issues vsv1.6.0-atlan),go build ./..., andgo test ./...with envtest 1.27.1.🤖 Generated with Claude Code