fix(replication): correct Postgres epoch constant (#4002)#4007
fix(replication): correct Postgres epoch constant (#4002)#4007gtx20060124-bot wants to merge 1 commit into
Conversation
…ad of 946080000000) (triggerdotdev#4002)
|
|
Hi @gtx20060124-bot, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughIn ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
| const now = Date.now() - 946080000000; | ||
| const now = Date.now() - 946684800000; | ||
| const upperTimestamp = Math.floor(now / 4294967.296); | ||
| const lowerTimestamp = Math.floor(now - upperTimestamp * 4294967.296); |
There was a problem hiding this comment.
🟡 Lower 32 bits of ACK timestamp computed in milliseconds instead of microseconds
The lowerTimestamp on line 663 computes the remainder of the 64-bit timestamp split in milliseconds instead of microseconds. The comment on line 660 states the timestamp should be "microseconds since midnight 2000-01-01", and the upper 32 bits are correctly derived from microseconds via now / 4294967.296 (which equals now * 1000 / 2^32). However, lowerTimestamp = Math.floor(now - upperTimestamp * 4294967.296) yields a value that is the remainder in milliseconds — it's ~1000× too small. The correct formula is Math.floor(now * 1000 - upperTimestamp * 4294967296) or equivalently Math.floor((now - upperTimestamp * 4294967.296) * 1000). You can verify by comparing the parsing logic at internal-packages/replication/src/client.ts:366 which correctly performs the inverse: upper * 4294967.296 + lower / 1000 — note the /1000 on the lower part, confirming the lower 32 bits are expected to be in microseconds. This pre-existing bug means the standby status update timestamp sent to PostgreSQL (used for pg_stat_replication.reply_time monitoring) can be off by up to ~71.6 minutes.
| const lowerTimestamp = Math.floor(now - upperTimestamp * 4294967.296); | |
| const lowerTimestamp = Math.floor((now - upperTimestamp * 4294967.296) * 1000); |
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #4002
The Postgres epoch constant was 7 days off (946080000000 vs correct 946684800000).
This affects keepalive timestamp decoding and standby status ack encoding.
Changed both occurrences to the correct value that matches pgoutput.ts.