[kriszyp]

⊙ Problem

Reconciled core removes _recordRelocate. Pro’s old fetch callback still calls it, which can close the retrieval socket after returning the read.

🤔 Is a local replica tier the right first step?

As decided on #436 (option B, step 1): reads leave placement unchanged; a fetched record is a disposable local copy. By-id tables and write forwarding are later steps.

💡 Solution

• On RocksDB, a read of a stub keeps the fetched record as a local replica (core REPLICA row). LMDB and replicaExpiration: 0 remain proxy-only; no read changes placement or logs a write.
• An owner update turns the replica back into a stub. An answer older than a stub that landed meanwhile is not kept.
• GET_RECORD answers a replica as not resident and names each row's writer; a base copy sends it as a bodyless stub selected by the peer capability, built from a cached list of stored index names.
• The deadline lives in core's per-table store and never crosses the wire (index): a replica expires to its stub after replicaExpiration (core default one hour), reads renew it, it survives restarts and never counts for replicatedConfirmation.

⚖️ Alternatives

• Fix the two defects inside the relocation write — every read would still be a placement write, which Replication W7: Robust sharding (residency-vs-routing split) #436 rejects.
• Skip replica rows in the base copy (LOCAL_ONLY) — the receiver would lose this node's index stub for the key; sending the stub projection keeps the copy equivalent to a stub holder's (flags).

🤔 Rollout: ship one release after #1087's send rule, or gate on a capability?

A replica is kept current only by the stubs every writer sends to every non-resident, which #1087 introduces. An older writer leaves a reader’s replica stale; hot reads renew its lifetime. Documented as "no earlier than the release after #1087"; a peer capability bit (#1097) would enforce this.

🤔 Match core’s demotion stub before selecting the frame

The copy-only projection includes the primary key and created time. The shared live-invalidation projection can return null and omit those fields. Reusing it alone would send a null legacy put for a key-only table. Core owns replica admission and demotion; Pro owns the peer-specific wire representation.

Design assessment

Framing-Verdict: chosen-approach-sound — preserve main’s capability replies, use core’s admission policy, and verify proxy behavior on both engines.

🔧 Changes

Product and architecture tour

Product and architecture tour

A read keeps a replica; nothing moves

Before	After
The GET_RECORD answer went to _recordRelocate: placement rerun with the reader first, a replicated relocate entry, a former resident demoted.	The answer goes to _storeReplica: RocksDB may keep a local REPLICA row under the stub’s identity; LMDB and zero-expiration tables remain proxies. No log entry or placement change.

The resolver resolves the read first, then hands the body to core. Its promise is checked only to decide whether received blobs stay.
Frame dispatch never waits for the row write, and a failed store is logged, not surfaced through the socket.

One read on a non-resident

1. Stub read — Replicator.load picks the lowest-latency resident and sends GET_RECORD.
2. Answer — the resident names the writer and record expiry; a stub or replica answers strict false to a learned residencyNotResident peer, otherwise legacy empty.
3. Resolve — the caller gets the record at once.
4. Keep or proxy — core may keep it on RocksDB unless a newer stub landed meanwhile; LMDB and zero-expiration tables decline it.
5. Invalidate — an owner write reaches the holder as the usual stub and overwrites the replica.
6. Expire — core's deadline store, renewed by reads, returns it to a stub with no replicated write.

Where to look: the resolver that hands the answer to core, the response handler that keeps received blobs the live row still uses or deletes them when the table is gone, and the responder that names the row's writer.

What a replica holder sends

A replica is this node's alone. Core’s _storeReplica consumes originNode to verify the writer on an equal-version answer. The GET_RECORD responder treats REPLICA like INVALIDATED (capability-gated not-resident reply), and the copy walk sends a replica as its stub projection (residencyStubOf, the same projection sendAuditRecord builds for non-residents) with its primary key and created time. A resident advertising residencyStubCopy gets invalidate plus the residency id; other residents get the legacy put without that id. That older receiver may store the stub as complete when it has no row, matching #1087. The replica body, blob flag and deadline stay local. Expiring rows retain main’s frame format: record expiry is omitted for both replicas and stored stubs. LOCAL_ONLY would skip the row and leave the receiver without its index stub.

Mixed versions

An older reader still relocates on its own reads and replicates relocate entries, which _writeRelocate applies as before. A new RocksDB reader fetching from an older owner may store a replica and the owner keeps its row. The REPLICA bit never crosses the wire. A node downgraded while holding replicas treats them as full rows and ignores their deadline store. Verify all replicas are demoted before downgrade; lowering replicaExpiration does not rewrite existing deadlines.

🤔 Replica-tier concerns before promotion

Core is pinned to harper main’s merge of #3375, 808d5f5438ad3651129bdd61a8945a03cbbafa64, which includes the PUT-before-MESSAGE defense (core #3499). LMDB deliberately stores no replicas.

Shared-file cleanup remains #1102. Declined stores, including LMDB proxies, use the existing 60-second blob deletion timer; slow-reader/blob-lifetime safety is untested here. Core’s by-id admission guard is not repeated after a pending blob save; publishing over a replica may also remove its non-resident flag. Qualify both before promotion. Resolver-cache and wire-decoder concerns still need native reproductions. Base copies omit record expiry for stored stubs too; expiring-row tests pin that parity.

✅ Verification

Build, typecheck, changed-file formatting and lint:required pass. Full Pro RocksDB units: 1,995 passing, two existing skips. Strict lint reports warnings in unchanged tests and main’s unassigned subscriptionRequest. Fixtures are removed by teardownHarper.

• Four-node residencyReplicas and residencyReads: 15 passing on RocksDB; nine passing on LMDB with seven replica-only skips. The schema, controls and config exercise replica storage and zero-expiration/LMDB proxy reads. After owners remove a proxied row, the reader returns 404.
• residencyReplicaWire, residencyWire and protocolCapabilities: 113 passing on RocksDB; 101 passing with 13 replica-only skips on LMDB. Tests await real proxy admission results and cover strict capability replies, unchanged stubs/history, and byte-identical copies before and after demotion for indexed, key-only and expiring rows.

Four-node cases

On RocksDB, a read keeps a replica and changes no residency or transaction log on any node. The replica then answers alone once the owners drop the record. A query stores replicas. An owner update returns the replica to a stub. A held GET_RECORD answer older than a stub that landed meanwhile is not kept, checked after that attempt's own store count increases. The replica expires back to a stub with no replicated write. replicatedConfirmation refuses to count the holder (RocksDB only). A replica survives a restart; the short-lived row is a stub afterward. The test does not isolate sweep re-arming from demotion before shutdown. Wire: a GET_RECORD answer names its writer. It is kept as a replica with no deadline on the row and no audit entry while the socket stays open. GET_RECORD answers a replica as not resident only to learned capable peers; other peers receive the legacy empty answer. The copy walk sends only its stub fields, selecting invalidation or the legacy put by peer capability, with no replica expiry.

Refs #436

Depends-on: HarperFast/harper#3375

Based on current main 626bbaff9faf34bf2551fb592dd568da39a64f50; core 808d5f5438ad3651129bdd61a8945a03cbbafa64.

Docs: HarperFast/documentation#746.

_{🤖 Generated by OpenAI Codex; posted via @kriszyp.}

Related PRs: #1115 overlaps, #1112 overlaps, #1087 overlaps, #940 overlaps, #554 overlaps, #1092 overlaps, #1103 overlaps, #1107 overlaps, #815 independent, #1116 overlaps, #1117 overlaps
Release target: v5.4 — existing PR milestone; new replica row semantics and core table options
Complexity: complicated

Deferred findings, each filed as an issue

Deferred findings

• 6cb0b3e01e180434 major performance — replication/replicationConnection.ts — Base copy projects and encodes a REPLICA row that sendAuditRecord discards and rebuilds for an excluded peer — NOT FILED
• dc5d761cf9d9dc5a major data-integrity — replication/replicationConnection.ts — Replica admission is default-on and ungated by peer capability; a pre-fix: preserve residency stubs across replication and remote reads #1087 writer leaves replicas stale, indefinitely for hot keys — NOT FILED
• 8e7953dc0cd5ac30 minor concurrency — replication/replicationConnection.ts — Retired copy can flush after awaited send resumes — NOT FILED

🤔 defer this PR's major performance finding 6cb0b3e01e180434 (Base copy projects and encodes a REPLICA row that sendAuditRecord discards and rebuilds for an exclu) to a separate issue instead of fixing it here?

🤔 defer this PR's major data-integrity finding dc5d761cf9d9dc5a (Replica admission is default-on and ungated by peer capability; a pre-#1087 writer leaves replicas s) to a separate issue instead of fixing it here?

Late Grok findings

2 surviving (highest major) — Grok reviewed a704a21

• major — replication/replicationConnection.ts:9745 — the GET_RECORD resolver calls _storeReplica on every fetched record, with no check of the peer's capability. A writer on a build from before fix: preserve residency stubs across replication and remote reads #1087 does not send stubs to non-resident peers, so a replica on a new reader is not updated by that writer's writes. A cold replica stays stale until its li
• nit — replication/replicationConnection.ts:641 — some added comments narrate code. Disputed in part: the comment at line 641 gives the reason for the per-table cache (the stub projection runs once per write per non-resident peer), which the code cannot say itself. This fold pass did not re-check the other added comments

Not standing: 1 separate-issue. Grok ran 15m (15m from launch to report); the fold adjudication (cursor-grok@a704a21bb62d-fc0d8e5c, claude) took 1m.

_{Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex,cursor-grok; late=cursor-grok:adjudicated; adjudicated=domain; declined=muse; rounds=10; full=5 @ a704a21}

_{Review-Attention: deep ~25m (critical: replicationConnection.ts; decisions: replica-default-on, mixed-version-gate, replica-answers-not-resident, detached-admission, do-less-alternative, release-target-unclear, deferred-6cb0b3e0, deferred-dc5d761c) @ a704a21}

[gemini-code-assist]

Code Review

This pull request implements local replicas of residency stubs to prevent relocation on reads, updating the replication design, connection handling, and adding comprehensive integration and unit tests. The feedback identifies several improvement opportunities, including guarding against a potential silent blob leak when a record is not found, refactoring a Promise constructor anti-pattern in _storeReplica, returning an empty object instead of null in residencyStubOf to avoid runtime errors, and adding parentheses to clarify bitwise operator precedence.

[gemini-code-assist]

If requestedTable is defined but primaryStore is nullish, or if getEntry(key) returns undefined (e.g., the record was deleted or not stored), collectRetainedFileIds(undefined) will be called. If collectRetainedFileIds throws on undefined inputs, the catch block will trigger and return early, causing a silent leak of all blobs in blobsToDelete.

We should guard the entry retrieval and only call collectRetainedFileIds if a valid entry exists. If no entry exists, retained remains undefined, allowing the subsequent loop to safely delete all blobs.

												let retained: Set<string> | undefined;
												try {
													const entry = requestedTable?.primaryStore?.getEntry(key);
													if (entry) {
														retained = collectRetainedFileIds(entry.value);
													}
												} catch {
													return; // the table is gone; its files are nobody's to reclaim here
												}

References

1. Do not blindly add optional chaining (?.) based on static analysis suggestions if prior guards or early returns already guarantee the object is defined. Also, avoid optional chaining if silently coalescing to undefined would violate invariants or cause unintended side effects (e.g., blanking a live value).

[gemini-code-assist]

Wrapping table._storeReplica(...) in new Promise<boolean>((kept) => kept(...)) is a Promise constructor anti-pattern. Since _storeReplica is an async function that already returns a Promise<boolean>, you can return it directly and chain the .catch(...) block directly on it.

Suggested change

	return new Promise<boolean>((kept) => kept(table._storeReplica(existingEntry, entry))).catch((error) => {
	return table._storeReplica(existingEntry, entry).catch((error) => {

[gemini-code-assist]

If table.indices is empty or only contains resolver-owned indexed names, residencyStubOf returns null. Passing null to storedFieldsOnly or encoder.encode is highly risky and can cause runtime TypeErrors (e.g., trying to access properties of null). Returning an empty object {} instead of null is much safer and represents a bodyless stub/replica perfectly.

Suggested change

	function residencyStubOf(table: any, record: any): Record<string, any> | null {
	let stub: Record<string, any> | null = null;
	for (const name in table.indices) {
	if (isResolverOwnedIndexedName(table, name)) continue;
	(stub ??= {})[name] = record[name];
	}
	return stub;
	function residencyStubOf(table: any, record: any): Record<string, any> {
	let stub: Record<string, any> | null = null;
	for (const name in table.indices) {
	if (isResolverOwnedIndexedName(table, name)) continue;
	(stub ??= {})[name] = record[name];
	}
	return stub ?? {};
	}

[gemini-code-assist]

Bitwise operators have lower precedence than comparison operators and can sometimes lead to subtle precedence bugs or readability issues. Adding explicit parentheses around entry.metadataFlags & INVALIDATED makes the evaluation order clear and unambiguous.

Suggested change

	const copyType = entry.metadataFlags & INVALIDATED || isReplica ? 'invalidate' : 'put';
	const copyType = (entry.metadataFlags & INVALIDATED) || isReplica ? 'invalidate' : 'put';