-
Notifications
You must be signed in to change notification settings - Fork 394
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
WT-13502 Add 'backup_checkpoint' to 'query_timestamp'. #11011
Conversation
Thanks for creating a pull request! The below questions and checklist are intended to help with verifying your change is well tested. Response is optional, but if you choose to respond please edit this comment. What makes this change safe?A good answer to this question helps the reviewers understand where they should focus their attention, so please consider these questions:
Checklist before requesting a review
|
LGTM. I didn't hear the discussion so I didn't mark it approved. |
Closing as I don't think this solution is relevant anymore. |
Reopening this PR because it is now relevant. |
@keitharnoldsmith @luke-pearson and @tod-johnson-mongodb this PR has been revived and is now part of the solution for the server code. It is now pulled to the current state of develop and ready for review. |
Test coverage is too low, this change probably shouldn't be merged as-is.
|
Right now the code will return a WT_TS_NONE timestamp if a backup cursor isn't opened. It could return an error instead. |
@keitharnoldsmith and @tod-johnson-mongodb can you please review this? It is real now and this PR has been reopened. |
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, i think one more comment in txn_timestamp.c
may be helpful. See https://github.com/wiredtiger/wiredtiger/pull/11011/files#r1801689373
Add a configuration setting to be able to query the timestamp that is the timestamp of the checkpoint pinned by an open backup cursor. @luke-pearson and @keitharnoldsmith here is an implementation of what we discussed. It basically works but I didn't go into situations where timestamps aren't in use or checkpoints sometimes taken without the stable or weird cases like that. This is the basic functionality and plumbing. But I did make a test!
Add a configuration setting to be able to query the timestamp that is the timestamp of the checkpoint pinned by an open backup cursor.
@luke-pearson and @keitharnoldsmith here is an implementation of what we discussed. It basically works but I didn't go into situations where timestamps aren't in use or checkpoints sometimes taken without the stable or weird cases like that. This is the basic functionality and plumbing. But I did make a test!