Maven resolution: cool down after HTTP 429, dedupe repositories by URI, and let a silent mirror keep the mirrored repository's policy - #8683
Merged
Conversation
A 429 is transient, so it is deliberately never negative-cached; but nothing remembered it either, so every subsequent metadata or POM lookup re-asked the throttled host, and a failed metadata request was followed by a second request for the directory listing. MavenExecutionContextView now keeps a host:port -> skip-until map next to the unreachable-endpoints set. sendRequest populates it on a 429 with a 60s cooldown; distinctNormalizedRepositories skips a cooling-down repository (reported through repositoryAccessFailedPreviously) and deriveMetadata does not follow a 429 with a listing request. Part 1 of #8682.
distinctNormalizedRepositories deduped on the post-mirror id, so two entries with different ids and the same URL (generated settings, a POM re-declaring a configured repository) cost a request each per lookup. Key on the URI that would be asked instead, with the trailing slash trimmed and the host compared case-insensitively; the first occurrence wins so order and credentials are preserved. The "central" id check stays on id, since that is Maven's override rule for the implicit Central. Part 2 of #8682.
…ored repository's MavenRepositoryMirror.apply set the mirrored repository's releases and snapshots to "true" unless the mirror explicitly said "false". Maven's DefaultMirrorSelector copies the mirrored repository's policies onto the mirror instead, so mirroring Central (releases-only) through a settings <mirror> should not make it a snapshot-accepting repository that joins every -SNAPSHOT lookup. When several repositories collapse onto one mirror, Maven unions their policies (DefaultRemoteRepositoryManager.mergeMirrors). The lazy repository iterator already yields the first occurrence that accepts the version being looked up, which is the same thing for a single lookup, but downloadMetadata was filtering by version only after deduplication and so would have dropped the snapshot-accepting mirrored repository in favour of the releases-only one. It now passes the version through to distinctNormalizedRepositories like download does. Part 3 of #8682.
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.
1. An endpoint that answers HTTP 429 is not asked again for 60s
A 429 is transient, so it is deliberately never negative-cached, but nothing remembered it either: every later metadata or POM lookup re-asked the throttled host, and a failed metadata request was followed by a second request for the directory listing.
MavenExecutionContextView.getThrottledEndpoints(): ahost:port -> skip-untilmap, sibling togetUnreachableEndpoints().MavenPomDownloader.sendRequestrecords a 429 with a 60s cooldown;HttpSenderResponseException.isThrottled()added.distinctNormalizedRepositoriesskips a cooling-down repository and reports it throughResolutionEventListener.repositoryAccessFailedPreviously;deriveMetadatadoes not follow a 429 with the listing request.Retry-Afteris not honored yet, as the issue anticipated.2. Repositories are deduplicated by URI, not by id
distinctNormalizedRepositoriesdeduped on the post-mirror id, so two entries with different ids and the same URL cost a request each per lookup. Theseenset is now keyed on the URI that would be asked (trailing slash trimmed, scheme/host case-folded); first occurrence wins so order and credentials are preserved. The"central"id check is untouched, since that is Maven's override rule for the implicit Central.3. A mirror without a policy of its own keeps the mirrored repository's
MavenRepositoryMirror.applywidened the mirrored repository toreleases=true, snapshots=trueunless the mirror saidfalse. Maven'sDefaultMirrorSelector.getMirrorcopies the mirrored repository's policies onto the mirror instead, so mirroring Central through a<mirror>no longer turns it into a snapshot-accepting repository. Explicit values on the mirror still override.Sections 2 and 3 are not quite independent
When several repositories collapse onto one mirror, Maven unions their policies (
DefaultRemoteRepositoryManager.mergeMirrors); the old widening was accidentally approximating that. With per-repository policy on the mirror, "first occurrence wins" would have kept releases-only mirrored-Central and dropped the snapshot-accepting mirrored repository.downloadalready passes the version into the iterator, so the first accepting occurrence wins — the union for a single lookup — butdownloadMetadatafiltered by version only after deduplication and would have stopped resolving-SNAPSHOTmetadata through a*mirror. It now passes the version through likedownloaddoes, and its redundant in-loop check is gone.mirrorKeepsThePolicyOfEachRepositoryItMirrorscovers exactly this case.Verification
:rewrite-maven:test: 1515 tests, 0 failures.rewrite-gradletests that go through mirrors (EffectiveGradleRepositoriesTest,FindRepositoryOrderTest,ChangeDependencyConcurrencyTest,UseJavaExtensionBlockTest): green.