Skip to content

fix(replication): raise default REPLICATION_BLOBTIMEOUT from 120000 to 900000ms #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kriszyp merged 1 commit into from
Jul 6, 2026
Merged

fix(replication): raise default REPLICATION_BLOBTIMEOUT from 120000 to 900000ms #520

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions replication/replicationConnection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@
// `replication_pauseStallTimeout` for clusters with extreme single-transaction sizes.
const PAUSE_STALL_THRESHOLD_MS =
env.get('replication_pauseStallTimeout') ??
Math.max(PING_TIMEOUT * 2, (env.get(CONFIG_PARAMS.REPLICATION_BLOBTIMEOUT) ?? 120000) * 2);
Math.max(PING_TIMEOUT * 2, (env.get(CONFIG_PARAMS.REPLICATION_BLOBTIMEOUT) ?? 900000) * 2);

/**
* Decide whether an idle replication connection should be terminated as dead.
Expand Down Expand Up @@ -451,7 +451,7 @@
logger.warn?.(`[test] forcing open-but-idle replication wedge for db "${databaseName}" (harper-pro#420)`);
ws.terminate = () => {};
ws.close = () => {};
ws._socket?.pause();

Check failure on line 454 in replication/replicationConnection.ts

View workflow job for this annotation

GitHub Actions / Unit Tests (Node.js v22) 

Property '_socket' does not exist on type 'WebSocket'.

Check failure on line 454 in replication/replicationConnection.ts

View workflow job for this annotation

GitHub Actions / Unit Tests (Node.js v24) 

Property '_socket' does not exist on type 'WebSocket'.

Check failure on line 454 in replication/replicationConnection.ts

View workflow job for this annotation

GitHub Actions / Unit Tests (Node.js v26) 

Property '_socket' does not exist on type 'WebSocket'.

Check failure on line 454 in replication/replicationConnection.ts

View workflow job for this annotation

GitHub Actions / Build Harper Pro (Node.js v24) 

Property '_socket' does not exist on type 'WebSocket'.

Check failure on line 454 in replication/replicationConnection.ts

View workflow job for this annotation

GitHub Actions / Build Harper Pro (Node.js v22) 

Property '_socket' does not exist on type 'WebSocket'.

Check failure on line 454 in replication/replicationConnection.ts

View workflow job for this annotation

GitHub Actions / Build Harper Pro (Node.js v26) 

Property '_socket' does not exist on type 'WebSocket'.
return true; // tell the watchdog to treat this connection's byte count as frozen
}

Expand Down Expand Up @@ -1529,7 +1529,11 @@
// drains to disk — so the watchdog can tell a healthy back-pressure pause (consumer draining) from a
// leg that died mid-pause (harper-pro#466). Only meaningful while paused.
let consumerProgress = 0;
const blobTimeout = env.get(CONFIG_PARAMS.REPLICATION_BLOBTIMEOUT) ?? 120000;
// Default 15min: a 120s cap dropped ~4,500 blobs to permanent divergence when a rolling
// upgrade + concurrent writes had blob transfers routinely stalling past the timeout, the
// receive watchdog then killed the subscription, and the audit cursor advanced past the
// missing blob. 900000 lets in-flight transfers complete across a peer restart window.
Comment on lines +1532 to +1535

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Increasing the default blobTimeout to 15 minutes (900,000 ms) has an unintended side-effect on the blobsTimer cleanup interval (defined at line 4438), which is set to run at blobTimeout interval.

With a 15-minute interval, any stalled blob stream in blobsInFlight can remain in memory for up to 30 minutes before being swept. Under high concurrent write load with stalling transfers, this slow reclamation can lead to significant memory accumulation (holding PassThrough streams and buffers) and potentially cause out-of-memory (OOM) issues.

Consider decoupling the blobsTimer sweep interval from blobTimeout (e.g., running the timer every 30 seconds or 1 minute) while keeping the expiration check against blobTimeout.

const blobTimeout = env.get(CONFIG_PARAMS.REPLICATION_BLOBTIMEOUT) ?? 900000;

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If REPLICATION_BLOBTIMEOUT is configured to 0 or a negative value, blobTimeout will be 0 or negative. This will cause setInterval(..., blobTimeout) for blobsTimer to run in a tight loop (as Node.js treats 0 or negative delays as 1 ms), pegging the CPU at 100%. Additionally, a negative blobTimeout would cause all blob streams to instantly time out.

Guarding blobTimeout at its definition ensures it is always positive and safe to use throughout the file.

Suggested change
const blobTimeout = env.get(CONFIG_PARAMS.REPLICATION_BLOBTIMEOUT) ?? 900000;
const rawBlobTimeout = env.get(CONFIG_PARAMS.REPLICATION_BLOBTIMEOUT) ?? 900000;
const blobTimeout = rawBlobTimeout > 0 ? rawBlobTimeout : 900000;

const blobsInFlight = new Map();
const outstandingBlobsToFinish: Promise<void>[] = [];
let outstandingBlobsBeingSent = 0;
Expand Down Expand Up @@ -1648,9 +1652,9 @@
// (reset on COPY_START and on each in-copy 'message'; stopped on copy finish / pause). On a stall it
// forces the same close-independent reconnect, which restarts the copy from the leader. (harper-pro#453)
copyProgressWatchdog = createReceiveWatchdog({
// blobTimeout (REPLICATION_BLOBTIMEOUT) defaults to 120000 and is shared with blobsTimer; guard
// blobTimeout (REPLICATION_BLOBTIMEOUT) defaults to 900000 and is shared with blobsTimer; guard
// against a misconfigured 0/negative that would otherwise forceReconnect in a tight loop.
intervalMs: blobTimeout > 0 ? blobTimeout : 120000,
intervalMs: blobTimeout > 0 ? blobTimeout : 900000,
Comment on lines +1655 to +1657

@gemini-code-assist gemini-code-assist Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

With blobTimeout now guaranteed to be strictly positive (by guarding it at its definition), we can simplify the copyProgressWatchdog interval configuration and remove the redundant inline guard.

Suggested change
// blobTimeout (REPLICATION_BLOBTIMEOUT) defaults to 900000 and is shared with blobsTimer; guard
// against a misconfigured 0/negative that would otherwise forceReconnect in a tight loop.
intervalMs: blobTimeout > 0 ? blobTimeout : 120000,
intervalMs: blobTimeout > 0 ? blobTimeout : 900000,
// blobTimeout (REPLICATION_BLOBTIMEOUT) defaults to 900000 and is shared with blobsTimer.
intervalMs: blobTimeout,

getBytesRead: () => copyProgressFrames,
onSilence: () => {
if (!inCopyMode || copyCompleteReceived) return; // only act on an actively-receiving, stalled copy
Expand Down Expand Up @@ -3767,7 +3771,7 @@
// receiver's apply consumer wedges at `lastReceivedStatus:"Receiving"` until its own idle
// watchdog fires (core/resources/blob.ts) 120s later. With the timeout, the catch below
// emits the finishing error frame so the receiver advances cleanly.
// Defaults ON to the replication blob timeout (REPLICATION_BLOBTIMEOUT, 120000 default) so a
// Defaults ON to the replication blob timeout (REPLICATION_BLOBTIMEOUT, 900000 default) so a
// stalled send can't silently wedge a base copy out of the box (harper-pro#453). The
// HARPER_BLOB_SEND_CHUNK_TIMEOUT_MS env var overrides it; set it to 0 to disable.
const rawEnv = process.env.HARPER_BLOB_SEND_CHUNK_TIMEOUT_MS;
Expand Down
Loading