-
Notifications
You must be signed in to change notification settings - Fork 3k
Add interfaces for Action CheckSnapshotIntegrity #10642
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
Conversation
Co-authored-by: Yufei Gu <[email protected]>
|
|
||
| /** | ||
| * Passes an alternative executor service that will be used for snapshot integrity checking. If | ||
| * this method is not called, snapshot integrity checker will still be running by a single |
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.
nit: 2nd part can be rewritten as a single threaded executor service is used by default
| * "/path/to/00001-8893aa9e-f92e-4443-80e7-cfa42238a654.metadata.json". | ||
| * @return this for method chaining | ||
| */ | ||
| CheckSnapshotIntegrity targetVersion(String targetVersion); |
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.
should the param name be changed to targetVersionFilePointer or targetVersionMetadataFile
|
I think this is one where we need to have a bit more discussion on the API. I think we've talked previously about how this would be helpful to have as a generic "Repair Snapshot" command which has a couple purposes.
Might be good to bring this up on the dev list for discussion |
|
Yea looks like there's some discussion in #10445 and also https://lists.apache.org/thread/6zvpcyvdzglh800v15mt7m7rt76ch3ol. Initially it makes sense to have a central procedure that can optionally do any of these things. |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
Co-authored-by: Yufei Gu [email protected]
Co-authored-by: Huaxin Gao [email protected]
This PR adds interfaces for Action CheckSnapshotIntegrity. We will have another PR to implement the interfaces in Spark 3.4 and Spark 3.5.