Skip to content
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

Update pgcopydb sentinel in the main follow process. #521

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

dimitri
Copy link
Owner

@dimitri dimitri commented Oct 30, 2023

To avoid infinite looping when endpos has been reached it's important to update our endpos value to the sentinel's one even in the main follow process.

In passing, review some error messages.

Also refrain from stopping early from reading data from a PIPE when a terminating signal is received, we should finish reading as per the comments in the code.

@dimitri dimitri added the enhancement New feature or request label Oct 30, 2023
@dimitri dimitri added this to the v0.14 milestone Oct 30, 2023
@dimitri dimitri self-assigned this Oct 30, 2023
@dimitri dimitri added bug Something isn't working and removed enhancement New feature or request labels Oct 30, 2023
To avoid infinite looping when endpos has been reached it's important to
update our endpos value to the sentinel's one even in the main follow
process.

In passing, review some error messages.

Also refrain from stopping early from reading data from a PIPE when a
terminating signal is received, we should finish reading as per the comments
in the code.
When the ENDPOS internal message is received from the PIPEs in replay mode,
check the current sentinel.endpos value on the source database to make sure
the ENDPOS message matches with the current setting, and that being the case
stop processing.
@dimitri dimitri force-pushed the fix/update-sentinel-in-follow-main-loop branch from 927c29d to dcc51ad Compare November 6, 2023 14:36
@dimitri dimitri merged commit 2a1c778 into main Nov 6, 2023
15 checks passed
@dimitri dimitri deleted the fix/update-sentinel-in-follow-main-loop branch November 6, 2023 14:40
@@ -990,6 +1014,13 @@ follow_wait_subprocesses(StreamSpecs *specs)
}
}

/* update current sentinel values (endpos) */
if (!follow_get_sentinel(specs, &(specs->sentinel), false))
Copy link
Contributor

@arajkumar arajkumar Nov 7, 2023

Choose a reason for hiding this comment

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

@dimitri I'm seeing lots of print(with -vv) with the latest version due to this. IIUC, now we call follow_get_sentinel for every 150ms?

play_lsn 0/0
2023-11-07 06:50:17 895197 SQL pgsql.c:490 Connecting to [source] "postgres://[email protected]:26479/defaultdb?sslmode=require&keepalives=1&keepalives_idle=10&keepalives_interval=10&keepalives_count=60"
2023-11-07 06:50:17 895197 SQL pgsql.c:1471 [SOURCE 4026526] select startpos, endpos, apply, write_lsn, flush_lsn, replay_lsn from pgcopydb.sentinel;
2023-11-07 06:50:17 895197 SQL pgsql.c:400 Disconnecting from [source] "postgres://[email protected]:26479/defaultdb?sslmode=require&keepalives=1&keepalives_idle=10&keepalives_interval=10&keepalives_count=60"
2023-11-07 06:50:17 895197

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes we do. We could call it less often I suppose, I will have a look, thanks for your feedback.

Copy link
Owner Author

Choose a reason for hiding this comment

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

See #531

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants