Skip to content

fix(replication): correct Postgres epoch constant in replication client #4433

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

Closed
deepshekhardas wants to merge 1 commit into from
Closed

fix(replication): correct Postgres epoch constant in replication client #4433

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
12 changes: 10 additions & 2 deletions internal-packages/replication/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ import {
PgoutputParser,
} from "./pgoutput.js";

/**
* Milliseconds between the Unix epoch (1970-01-01) and the Postgres epoch
* (2000-01-01), used to convert between Postgres replication timestamps and
* `Date.now()`. Mirrors `(POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) * USECS_PER_DAY`
* in pgoutput.ts (946684800000000 micros).
*/
const POSTGRES_EPOCH_MS = 946684800000;
Comment on lines +15 to +21

@devin-ai-integration devin-ai-integration Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 No .server-changes note for a server-affecting fix

internal-packages/replication ships only inside the webapp/server image (consumed by apps/webapp/app/services/runsReplicationService.server.ts and sessionsReplicationService.server.ts), so per AGENTS.md / CONTRIBUTING.md a .server-changes/ entry may be expected for server-only changes. The rule text enumerates apps/webapp/, apps/supervisor/, "etc.", so whether internal packages qualify is ambiguous; a maintainer should confirm. The behavioral impact is limited to the heartbeat timestamp value emitted to listeners, and neither consumer reads it (both destructure only lsn and shouldRespond), so the fix has no user-visible effect today.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


export interface LogicalReplicationClientOptions {
/**
* The pg client config.
Expand Down Expand Up @@ -521,7 +529,7 @@ export class LogicalReplicationClient {
} else if (buffer[0] === 0x6b) {
// Primary keepalive message
const timestamp = Math.floor(
buffer.readUInt32BE(9) * 4294967.296 + buffer.readUInt32BE(13) / 1000 + 946080000000
buffer.readUInt32BE(9) * 4294967.296 + buffer.readUInt32BE(13) / 1000 + POSTGRES_EPOCH_MS
);
const shouldRespond = !!buffer.readInt8(17);
this.events.emit("heartbeat", { lsn, timestamp, shouldRespond });
Expand Down Expand Up @@ -833,7 +841,7 @@ export class LogicalReplicationClient {
const slice = lsn.split("/");
let [upperWAL, lowerWAL]: [number, number] = [parseInt(slice[0], 16), parseInt(slice[1], 16)];
// Timestamp as microseconds since midnight 2000-01-01
const now = Date.now() - 946080000000;
const now = Date.now() - POSTGRES_EPOCH_MS;

@devin-ai-integration devin-ai-integration Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Standby status low-order timestamp word is still scaled in milliseconds, not microseconds

The acknowledge path builds a Postgres standby status update whose timestamp field must be microseconds since 2000-01-01, split into two 32-bit words. upperTimestamp correctly computes micros >> 32 (now / 4294967.296), but lowerTimestamp is computed as now - upperTimestamp * 4294967.296, which is still in milliseconds — it should be multiplied by 1000 to yield the low 32 bits of the microsecond value (internal-packages/replication/src/client.ts:846). The result is a reply timestamp that can be off by up to ~71 minutes in pg_stat_replication.reply_time. This is pre-existing and unrelated to the epoch constant fix, but sits directly adjacent to the changed line; worth fixing in the same pass since the PR is about timestamp correctness.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const upperTimestamp = Math.floor(now / 4294967.296);
const lowerTimestamp = Math.floor(now - upperTimestamp * 4294967.296);
if (lowerWAL === 4294967295) {
Expand Down