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

GEODE-3800: Replace BackupManager with BackupService #1372

Merged
merged 4 commits into from Feb 1, 2018

Conversation

agingade
Copy link

For each backup request, instead of creating BackupManager, the request is passed to a BackupService.

Copy link

@nreich nreich left a comment

Choose a reason for hiding this comment

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

Looks good to me

@@ -192,7 +192,7 @@ boolean isCancelled() {
return isCancelled;
}

void waitForBackup() {
void waitTillBackupFilesAreCopiedToTemporaryLocation() {
Copy link

Choose a reason for hiding this comment

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

The name suggests too much of the internal implementation. Maybe something like: "waitUntilDestroysAllowed" or "waitForFileCopyCompletion", something like that?

Copy link
Author

Choose a reason for hiding this comment

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

It was done only on BackupTask...The reason was to help the reader to know when the wait will terminate.

Copy link
Contributor

@kirklund kirklund left a comment

Choose a reason for hiding this comment

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

+1

@agingade agingade merged commit 0f6e09c into apache:develop Feb 1, 2018
@agingade agingade deleted the feature/GEODE-3800 branch February 1, 2018 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants