Repository navigation
docs(compose): correct what the startMx comment says about #12846 - #14399
Merged
Merged
Conversation
docker#12846 was not a race between container starts but a flaw of the daemon's port allocator, fixed in Engine 28.3.0: the mutex added as its workaround could not help, and the comment said the opposite. Say what the mutex does and does not provide, and that the sequential start of a service's replicas (docker#8530) is the plan's replica chain, not this mutex. Also fix the comment of the test of a stale non-relay container of a provider-backed service, which still described a special case the start path no longer has: planServiceStart routes every provider to planProviderStart, which decides bare-vs-enriched per container. Comments only: the token stream of every Go file, comments excluded, is unchanged. Part of docker#14081 Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
…eview Say that a relay restarted by a start plan does take startMx while the deployment of the relay does not, that the lock was unlikely (rather than unable) to help against docker#12846, and name isReplicaRelay, which is what gates pre_start. Rewrap the test comment. Comments only. Part of docker#14081 Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
24 of 25 tasks
dock-tor
approved these changes
Oct 27, 2026
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
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.
What this PR does, in one sentence
Corrects the comment of
startMx, which blamed #12846 on concurrent container starts, and one test comment that described a special case the start path no longer has — comments only, no code.Context
The comment of
startMxsays that, with Engine 28.0 to 28.2, "containers started concurrently could be assigned the same published host port". That is not what #12846 was: a container with several random published ports, one on0.0.0.0and one on a specific address, could be given the same host port twice, a flaw of the daemon's port allocator fixed in Engine 28.3.0 (moby/moby#50054). As noted on the issue, it was not a race between starts, so the mutex added as a workaround by #12851 was unlikely to help. The sequential start of a service's replicas, which #8530 (a port range shared by the replicas) needed and #10067 introduced, is a different thing, and today it is the plan's replica chain, not this mutex.The comment of
TestPlanStart_ProviderPathEnrichesStaleNonRelayContainersays the start path "only special-casesProvider != nilfor the zero-containers case".planServiceStartactually routes every provider toplanProviderStart, and the bare-vs-enriched start is decided per container.What the PR brings
startMxcomment states what the mutex is — a workaround for a daemon flaw that has been fixed — what it does not provide (the replica ordering, which comes from the plan), which paths take it (the plan executor, a relay restarted by a start plan included) and which do not (restart, run, the deployment of the relay, the hook runners). The TODO to remove it once Engine 28.x is out of support and the paragraph on why it is a package variable are kept, the latter reworded now that no race is claimed.isReplicaRelay, which is what gatespre_start.Why this is no functional change
//comment line.pkg/,cmd/andinternal/, comments excluded, is identical before and after.mainand from this branch have identical symbol tables (names, sizes, types).go vet,gofmt, the linter (only the known findings) andgo test ./pkg/compose/ -racepass.Part of #14081 (Lot 3).
🤖 Generated with Claude Code