-
Notifications
You must be signed in to change notification settings - Fork 53
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
Restore option for cluster type #324
Restore option for cluster type #324
Conversation
Can one of the admins verify this patch? |
@mesosphere-ci retest this please |
1 similar comment
@mesosphere-ci retest this please |
@mesosphere-ci retest this please |
1 similar comment
@mesosphere-ci retest this please |
@mesosphere-ci retest this please |
final String destinationFile = cfDir.getAbsolutePath() + fileKey.substring(fileKey.lastIndexOf("/")); | ||
downloadFile(tx, bucketName, fileKey, destinationFile); | ||
LOGGER.info("Keyspace {}, Column Family {}, FileKey {}, destination {}", keyspace, columnFamily, fileKey, destinationFile); | ||
if (ctx.getRestoreType().equals("new")) { |
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: Use Objects.equals()
.
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.
will fix in next PR of restore schema.
@@ -79,6 +82,16 @@ public void setAzureKey(String azureKey) { | |||
this.azureKey = azureKey; | |||
} | |||
|
|||
public String getRestoreType() { | |||
if (restoreType != null && !restoreType.isEmpty()) { |
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: Maybe use StringUtils.isNotBlank(restoreType)
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.
will fix in next PR of restore schema.
@@ -108,6 +122,10 @@ private boolean isValidAzureRequest() { | |||
return azureAccount != null && azureKey != null && externalLocation.startsWith("azure:"); | |||
} | |||
|
|||
private boolean isValidRestoreType() { | |||
return restoreType == null || restoreType.isEmpty() ? true: restoreType.matches("existing|new"); |
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.
Why is a null OR empty restoreType valid ?
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.
BackupRestoreRequest object is used for backup and restore request, and for backup request, restoretype can be set null or empty.
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.
Okay. Got it.
For restore, added a new option to restore on existing cluster with same token range or a completely new cluster bootstrapped from scratch. For same cluster, use nodetool refresh to restore data and for new cluster use sstableloader. Difference is due to the significant performance disparity.