You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Keep Gantry provider records fresh and reject stale DHT hits - #749
A peer can remain in Gantry's discovery records after it loses a layer. Stale entries can hide healthy peers beyond the first 20 results, or make a pull wait for a peer that cannot serve the content. A missing layer could also be reported as "busy" when the server was at capacity.
What changed
Refresh discovery records every 3 hours, expire them after 6 hours, and stop refreshing content that is no longer present.
Look beyond the first 20 providers during normal pulls and recovery checks.
Check that a peer has the requested layer before treating it as available or busy. Missing content returns 404 even when the server is full.
When all advertised peers are unusable, return 503 before starting the response so containerd can try origin instead of waiting on stale peers.
Genuinely busy peers still wait for capacity, and empty discovery still uses the existing cold-start flow. This PR does not change lease retention or add an end-to-end recovery deadline.
Rebased #749 onto main as one commit:
- Set provider-record validity to 6h with a sweeping reprovide every 3h
(max 10m delay); stop reproviding digests containerd no longer holds.
- Cold-start chair polling and the NF5 recheck treat DHT records as
candidates; a provider must answer a bounded metadata HEAD to count.
- Carry the DHT settings into the Helm chart configmap, which replaced
deploy/gantry/configmap.yaml.tmpl on main.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stale provider records could fill kad-dht's 20-provider result, hide
usable providers, and hold containerd for the whole re-discovery budget.
- Discovery streams FindProvidersAsync with no count, which also skips
kad-dht's local-store shortcut, behind a new optional
ifaces.ProviderStreamer. The lookup stays bounded by the 2s budget.
- The mirror tries each streamed provider once, skipping known-bad ones,
and no longer sends response headers before round 0.
- After the walk: busy providers send headers and wait as before; no
advertised provider sends headers then cold-starts; advertised but
unusable providers return 503 at once so containerd pulls from origin.
- maxPeerAttempts <= 0 now means no per-round cap.
- Update the stale-provider recovery design and the v1 transfer policy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use streamed discovery across recovery paths, check content before capacity, require peer metadata before busy headers, and discard stale busy signals. Add DHT and containerd-fetcher regressions and preserve containerd-owned origin fallback in the eviction e2e test. No end-to-end recovery deadline is added.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Confirm each busy candidate directly before counting its capacity signal and flush waiting headers before slower candidates. Preserve verified busy results through partial discovery errors and unrelated peer failures. Strip registry credentials from metadata probes and verify whole-digest metadata while resuming.
Add regressions for the three Copilot findings and metadata failure cases. Keep the existing cold-start and busy-wait timing policy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid redundant initial publication for already-published keys
internal/gantry/discovery/discovery.go:510
StartProviding(false, ...) also queues an initial publication for new keys when the sweeper is online; it is not registration-only (go-libp2p-kad-dht v0.42.2, provider/provider.go:1331-1370). The synchronous h.d.Provide(ctx, c, true) above has already published the digest, so this adds redundant publication work for newly advertised content, including startup inventory. Register already-published keys in the sweeper's keystore and schedule without queuing another initial publication, or use one publication path while preserving the synchronous success contract.
Report discovery lookup latency independently of candidate consumption
internal/gantry/mirror/mirror.go:2311
Discovery continues concurrently with each fetch, so subtracting the entire fetch duration also subtracts time spent performing the lookup. For example, an immediately emitted candidate followed by a two-second lookup deadline during a five-second fetch records almost zero lookup latency. This feeds p2p_dht_lookup_duration_seconds (cmd/gantry/main.go:616-618), hiding slow lookups. Capture query completion time in discovery and report its actual elapsed duration independently of candidate consumption.
Bound per-peer HEAD requests within the overall probe deadline
Each HEAD inherits the entire caller deadline. Four peers that accept connections but never send response headers can occupy every probe slot until cancellation, leaving a healthy fifth provider untried. The production client has a one-hour request timeout and no response-header timeout (transfer/client.go:69-98). This can exhaust the three-second fallback recheck (cmd/gantry/main.go:939-942) or a chair polling window (coldstart/chair.go:675-684). Bound each HEAD separately, leaving time within the overall budget to try queued candidates, and add a regression with four stalled peers followed by a healthy one.
Keep each Provide or Withdraw operation and its announced-set update under the same per-digest gate across notifications and reconciliation. Honor existing operation budgets, preserve parallelism for unrelated digests, and release idle gate entries.
Add deterministic overlap regressions in both directions plus cancellation, failure, and independent-digest concurrency coverage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests for periodic refresh and remote provider expiration
internal/gantry/discovery/discovery.go:404
Please add short-interval, multi-peer tests for periodic refresh and remote expiration. The lifecycle test checks only schedule membership (discovery_test.go:191-210), so it would pass even if refresh never published or configured validity were ignored. Verify that a live provider remains remotely discoverable across multiple validity windows, then that its remote record expires after Withdraw stops refreshes.
Avoid redundant DHT publication after synchronous announcement
internal/gantry/discovery/discovery.go:510
StartProviding(false, ...) queues an immediate publication for newly registered keys; it is not registration-only. In kad-dht v0.42.2, handleProvide stores new keys, enqueues them, and starts provideLoop (provider/provider.go:1331-1370). Line 502 already published the digest synchronously, so initial announcements, including startup inventory reconciliation, trigger redundant DHT publications. Preserve the synchronous success contract while registering the key in the sweeper's keystore and schedule without another immediate publication.
Add per-peer HEAD timeouts to prevent probe slot exhaustion
Each HEAD inherits the whole caller deadline, with no shorter per-peer timeout. Chair polling passes its full stall window (coldstart/chair.go:675-684), and the production client has a one-hour request timeout and no response-header timeout (transfer/client.go:69-98). Four peers that keep connections alive but never answer HEAD can therefore occupy all probe slots until the polling window expires, leaving a healthy fifth provider untried. Give each HEAD a short child timeout that releases its slot before the overall budget expires, and test four blocked probes followed by a usable provider.
Preserve synchronous initial DHT publication, then store the key and schedule periodic reprovide without another immediate or bootstrap publication. Own and close the in-memory sweeper keystore explicitly.
Bound each shared metadata HEAD to two seconds under its caller deadline so four stalled peers cannot hide a healthy fifth. Add regressions that fail against the previous runtime code and pass with these fixes. No new configuration or overall recovery deadline is introduced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressed all three additional items in the Copilot review body:
Initial DHT publication stays synchronous. Keys are then stored and scheduled for periodic reprovide without queuing another initial or bootstrap publication.
Each shared metadata HEAD has its own two-second timeout within the existing caller deadline, allowing a healthy fifth provider to be tried after four blocked probes.
A real two-peer test now proves that periodic refresh keeps a provider in a remote DHT store across three validity windows, then that the remote record expires after withdrawal. It reads the remote store passively, so lookups cannot recache the provider and hide expiration.
Runtime fixes: 7c9a83c. Additional network coverage: 0c22f88. The coverage tests passed three consecutive race-enabled runs. All 28 Gantry-related test packages passed under the race detector for the runtime fixes; lint and build passed. Generated NOTICE was also refreshed and verified.
No new settings or overall recovery deadline were added. The Go-only review branch is synced with the runtime source and intentionally excludes tests, docs, and dependency manifests.
Regenerate NOTICE after the existing datastore dependency became a direct import. Verified with make notice-check.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover the remaining test-coverage request in the Copilot review body with a real two-peer DHT test. Prove periodic refresh keeps a provider in a remote store across three validity windows, then prove the record expires after withdrawal without lookups recaching it.
Reuse the connected-provider fixture for the existing lifecycle regression. The focused tests passed three consecutive race-enabled runs. No runtime behavior changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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.
Problem
A peer can remain in Gantry's discovery records after it loses a layer. Stale entries can hide healthy peers beyond the first 20 results, or make a pull wait for a peer that cannot serve the content. A missing layer could also be reported as "busy" when the server was at capacity.
What changed
Genuinely busy peers still wait for capacity, and empty discovery still uses the existing cold-start flow. This PR does not change lease retention or add an end-to-end recovery deadline.