-
Notifications
You must be signed in to change notification settings - Fork 43
Fix Z0DAN sync origin advancement to use slot creation LSN #302
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.
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
Open
rasifr
wants to merge
2
commits into
pgEdge:main
Choose a base branch
from
rasifr:task/SPOC-394/zodan-lsn-fix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
Member
|
Can you please also include |
Member
Author
Done. |
65e12fc to
1396792
Compare
When adding a third node in a Z0DAN sync scenario, the origin advancement
logic in spock.check_commit_timestamp_and_advance_slot was using
spock.lag_tracker to retrieve commit timestamps and convert them back to LSNs.
This approach no longer works because spock.progress is now an in-memory HTAB
instead of a catalog table, so lag_tracker doesn't retain historical
data after the SYNC process COPY operation.
Root Cause:
The procedure spock.create_disable_subscriptions_and_slots creates logical
slots on existing nodes (e.g., n2) when adding a new node (n3). In v5,
the commit LSN/timestamp from the source node (n1) was copied to n3 via
lag_tracker during SYNC, and spock.check_commit_timestamp_and_advance_slot
would use this to advance the origin. With the HTAB-based progress table, this
data is no longer available after COPY.
The Fix:
1. Capture the LSN returned by pg_create_logical_replication_slot in
create_disable_subscriptions_and_slots and store it in temp_sync_lsns
2. Use this LSN directly in check_commit_timestamp_and_advance_slot to
advance the origin, eliminating the dependency on lag_tracker and the
timestamp→LSN conversion
This approach is more reliable because it uses the authoritative LSN from
slot creation - the exact point where the apply/sync process will begin
decoding when the subscription is enabled.
Apply the same fix from commit 86acd7b to zodan.py: - Use LSN from pg_create_logical_replication_slot() - Advance slot to stored commit LSN instead of querying lag_tracker - Store both sync_lsn and commit_lsn for later use
1396792 to
1db124d
Compare
ibrarahmad
approved these changes
Jan 9, 2026
Contributor
ibrarahmad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
mason-sharp
reviewed
Jan 9, 2026
| else: | ||
| # Parse the result to extract LSN (format: "slot_name|lsn") | ||
| parts = result.split('|') | ||
| lsn = parts[1].strip() if len(parts) > 1 else None |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just select lsn in the query on line 413 and simplify?
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.
When adding a third node in a Z0DAN sync scenario, the origin advancement
logic in spock.check_commit_timestamp_and_advance_slot was using
spock.lag_tracker to retrieve commit timestamps and convert them back to LSNs.
This approach no longer works because spock.progress is now an in-memory HTAB
instead of a catalog table, so lag_tracker doesn't retain historical
data after the SYNC process COPY operation.
Root Cause:
The procedure spock.create_disable_subscriptions_and_slots creates logical
slots on existing nodes (e.g., n2) when adding a new node (n3). In v5,
the commit LSN/timestamp from the source node (n1) was copied to n3 via
lag_tracker during SYNC, and spock.check_commit_timestamp_and_advance_slot
would use this to advance the origin. With the HTAB-based progress table, this
data is no longer available after COPY.
The Fix:
This approach is more reliable because it uses the authoritative LSN from
slot creation - the exact point where the apply/sync process will begin
decoding when the subscription is enabled.