reliability: reconstruct DNS single-flight on protected main - #205
reliability: reconstruct DNS single-flight on protected main#205seonghobae wants to merge 36 commits into
Conversation
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
Integrate protected main 1d2e190 without rebasing. Keep the current DNS source/tests/research delta and take protected-main CHANGELOG as the merge resolution so the integrated documentation contract can be revalidated independently.
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review finding
MAJOR — A short-deadline flight owner can prematurely fail longer-deadline callers
_resolve_all_global_addresses() registers the shared authority flight before acquiring _DNS_RESOLUTION_SLOTS, then uses only the first caller's dns_timeout_seconds for that slot acquisition. If the first caller has a short deadline and the global resolver pool is temporarily full, it stores a shared error and completes the flight. A second caller that already joined the same authority with a longer remaining deadline is woken and denied immediately, even though its own deadline still permits waiting for capacity and performing the lookup.
That contradicts this PR's documented contract that each caller retains its own finite deadline. The path is fail-closed, so it does not widen egress authority, but it creates cross-caller availability coupling and makes the result depend on which caller wins ownership.
Fix direction: make pre-worker capacity exhaustion a retryable flight outcome for non-owner waiters, or otherwise decouple shared-flight creation from the first caller's slot-acquisition budget. The owner whose deadline expires must still fail. A waiter with remaining time must be able to re-enter the registry, become the next owner, acquire a later-available slot, and complete one DNS lookup. Preserve one live resolver per authority, the finite global slot ceiling, no completed-result cache, generic errors, and per-caller address-policy validation.
Regression: orchestrate a short-deadline owner and a longer-deadline joiner while a controlled slot provider rejects the first acquisition and permits the next. Assert the short caller receives the generic denial, the long caller succeeds, exactly one getaddrinfo call runs, the slot is balanced, and the flight registry is empty. Add the async-equivalent case or prove the shared synchronous core covers it under the existing async deadline wrapper.
|
@opencode-agent fix Address the current-head review finding on |
Purpose
Carry issue #108's live-only DNS single-flight reliability/security boundary onto the exact current protected
mainwithout transferring stale predecessor evidence. This is the current-main replacement for #156.Exact identity
7faf7a3b8a47980113982914000e724ab6a6cda5;f95f75747a555b95a44879f09f28489ee6d41121;39332783589c8571f15d77ea60170d918fb82461;39332783589c8571f15d77ea60170d918fb82461, with first parent reliability: reconstruct DNS single-flight on integrated validation-result boundary #156 and second parent protected main;CHANGELOG.md,docs/research/dns-resolution-resource-bounds.md,src/egressweave/validation.py,tests/test_dns_resolution_singleflight.py,tests/test_dns_resolution_singleflight_documentation.py,tests/test_dns_singleflight_docstrings.py,tests/test_dns_timeout.py, andtests/test_dns_worker_start_failure.py.The integrated tree preserves current protected-main release workflow, release runbook, release evidence contract test, and release changelog entry while retaining the validated DNS
[Unreleased]entry.Runtime contract
One canonical
(hostname, port)may share only one live raw DNS lookup. Each caller retains its own finite deadline and independently reapplies address scope, local-development policy, deduplication andmax_resolved_addresses. Completed results are not cached. Worker-start failures release the resolver slot and remove the live flight without swallowingKeyboardInterrupt/SystemExit; public ordinary failures remain behind the generic denial boundary.Evidence discipline
Historical #156 checks, reviews, mergeability, and base identity do not transfer to this head. Keep Draft until hosted exact-head CI/security/SAST complete, all current-head findings are addressed, actual immutable-pinned Dependency Review executes successfully rather than being skipped, and any active ruleset-required Strix evidence is substantive rather than wrapper-only/backend-unavailable. Do not self-approve, bypass, Admin-merge, or treat queued/skipped/status/model/predecessor evidence as passing.
Progresses #108. Supersedes #156 after this replacement is observable and #156 is closed as historical predecessor.