Repository navigation
Avoid false 404s when residency holders have only stubs - #1112
Merged
Merged
Conversation
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
…rtions Use a separate single-holder missing key to establish a real empty reply, then repeat the stopped-holder read. Tighten strict-false receiver coverage and capability expectations, and describe the enduring request-timer limitation. Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Retain main’s deletion frame for all requesters. Cover deletion before an older live or unreachable holder through the sender, receiver and loader for stored and ID-derived lists. Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: hp-residency-not-resident-answer Co-Authored-By: GPT-5 Codex <noreply@openai.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a new protocol capability, residencyNotResident, which allows the GET_RECORD replication request to distinguish between a "not resident" response (returning false) and an "absence" response (returning an empty response). This prevents a stale residency list or empty holder from incorrectly reporting absence when a record is actually present on an unreachable or offline node. The changes include updates to replication design documentation, capability negotiation, connection handling, replicator loading logic, and comprehensive integration and unit tests to verify the new behavior. No review comments were provided, so there is no feedback to evaluate.
This was referenced Oct 23, 2026
kriszyp
marked this pull request as ready for review
October 23, 2026 21:01
This was referenced Oct 24, 2026
Merged
Closed
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.
⊙ Problem
A stale residency list can name only stub holders. Their empty GET_RECORD answers look like missing records, so a read returns 404 while full copies still exist elsewhere.
🤔 Versioned answers settle; empty replies need every holder
A versioned live record or deletion settles immediately. Otherwise, absence needs every non-self holder’s answer and at least one unversioned empty. This prevents false 404s during partial outages but can turn ID-derived misses into 502s.
💡 Solution
Peers advertise
residencyNotResident. Capable requesters receivefalsefor stubs; missing rows answer with an unversioned empty; deletions retain main’s versioned tombstone frame. For stubs and missing rows, incapable or unlearned requesters receive the legacy empty frame byte-for-byte. Exhausting only non-resident holders produces a descriptive 502.⚖️ Alternatives
Framing-Verdict: better-alternative-exists, resolved: aggregate weak empties across all holders; accept versioned answers immediately.
🤔 Keep the compatibility gate
Legacy peers require identical frames, so the marker has its own capability. Retrying old empty replies cannot distinguish stubs from absence.
🤔 Retain sequential holder requests
Parallel GET_RECORD requests would also start parallel relocations, so requests stay sequential. Isolated main/HEAD loader measurements over loopback WebSockets with 10 ms delayed replies gave 11/21/31 ms for 1/2/3 empty holders, versus main’s 11 ms. A 100 ms second holder raised the miss to 111 ms; a first-holder deletion still took one request. These measure the mechanism, not production latency. Follow-up: #1114.
🤔 Keep the design wording
The notes describe terminal versioned answers, network-error causes and legacy empties; further stylistic rewrites would not change that contract.
🔧 Changes
Product and architecture tour
Responses and holder selection
The capability registry advertises and normalizes the new key without changing the protocol version. The WebSocket responder gates only the stub marker and keeps the existing versioned null tombstone. The receiver passes through strict
falsewithout decoding or relocation. Stored and ID-derived lists both try the next holder after negative answers. Network errors remain the final 502's cause.The Pro design notes and root index state the response contract. The merged core companion qualifies legacy stub copy: peers lacking learned
residencyStubCopysupport receiveputwhen listed or without a residency list; excluded peers follow normal routing. Its gitlink update is separate.#1113 tracks the requirement that #1101 extend the gated answer to
REPLICArows and preserve the false relocation guard; this base has no such flag. #1092 overlaps the capability registry. Older responders still answer stubs the same as absence. Session/reconnect work (#1083–#1085) remains separate.✅ Verification
Executed checks and baseline failures
PR QA: skipped @ dc21989 — this one-task dispatch prohibits sub-agent delegation
Refs #436.
Depends-on: HarperFast/harper#3475
🤖 Generated by OpenAI Codex; posted via @kriszyp.
Release target: v5.4 — follows the linked issue and residency fixes.
Related PRs: #1092 overlaps, #1101 overlaps, #1087 overlaps, 2 others independent
Complexity: complicated
Follow-ups #1113 and #1114 are open and pass the readiness check; the receipt has not yet linked #1114.
Deferred findings
731730d933ac1561minor performance —replication/replicator.ts— Sequential id-derived misses increase latency and fail when a listed holder is unavailable — NOT FILED2d93f19e8672063fnit documentation —replication/DESIGN.md— Clarify future REPLICA compatibility when integrating PR 1101 — filed Reconcile REPLICA not-resident documentation when #1101 integrates #1113Dispatch: task
hp-residency-not-resident-answer· queued by unknown · ran by codex/gpt-6-astra/xhigh · worker kzyp-xps-1Review-Coverage: authored=codex; ran=gemini,claude; adjudicated=domain; declined=cursor-grok,cursor-composer,muse; rounds=4; full=4 @ dc21989
Review-Attention: study ~15m (critical: protocolCapabilities.ts, replicationConnection.ts +1; decisions: do-less-alternative, absence-evidence, terminal-versioned-answer, negative-read-policy, sequential-holder-requests) @ dc21989