-
Notifications
You must be signed in to change notification settings - Fork 502
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
HDDS-11285. cli to trigger quota repair and status #7104
Conversation
@sumitagrawl , is it allowed to submit multiple quota repair commands in the same time for different buckets? |
5ecd2a4
to
ee71463
Compare
No, its not allowed. As this operation is resource intensive, so allowed one quota repair task at a point of time. Since this is not a normal operation, so its fine to have one request at a time. |
@@ -279,6 +279,8 @@ public static boolean isReadOnly( | |||
case PrintCompactionLogDag: | |||
case GetSnapshotInfo: | |||
case GetServerDefaults: | |||
case QuotaRepairStatus: | |||
case QuotaRepairTrigger: |
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.
QuotaRepairTrigger -> startQuotaRepair
QuotaRepairStatus -> getQuotaRepairStatus
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.
Updated
@@ -151,6 +151,8 @@ enum Type { | |||
ListOpenFiles = 132; | |||
QuotaRepair = 133; | |||
GetServerDefaults = 134; | |||
QuotaRepairStatus = 135; |
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.
QuotaRepairStatus -> getQuotaRepairStatus
QuotaRepairTrigger -> startQuotaRepair
Most command enum names follow the "action" + "target" pattern. Let's keep the same pattern. Please also rename the request and response message name accordingly.
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.
Updated
// lock in progress operation and reject any other | ||
if (!IN_PROGRESS.compareAndSet(false, true)) { | ||
LOG.info("quota repair task already running"); | ||
return CompletableFuture.supplyAsync(() -> false); | ||
throw new OMException("Operation in progress", OMException.ResultCodes.QUOTA_ERROR); |
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.
Can you change the exception message to something like "There is a quota repair task already running. Please try it later"
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.
Updated as "Quota repair is already running"
@@ -279,6 +279,8 @@ public static boolean isReadOnly( | |||
case PrintCompactionLogDag: | |||
case GetSnapshotInfo: | |||
case GetServerDefaults: | |||
case GetQuotaRepairStatus: | |||
case StartQuotaRepair: |
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.
This StartQuotaRepair is a write operation.
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.
It trigger a repair activity, but not directly a write operation to ratis --> and as part of this, it further trigger a write operation to ratis based on repair to be done.
Since can not be submitted to Ratis as no direct write action, so path of Read is followed.
This is similar to "RangerBGSync", "SnapshotDiff" where trigger happens.
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.
I see, thanks for the explanation.
The last patch LGTM. Thanks @sumitagrawl. One further suggestion is can we display the lastRunFinishedTime and lastRunStartTime in a human readable format? |
Handled, now output sample,
|
341c6b6
to
5382f79
Compare
What changes were proposed in this pull request?
CLI option to trigger repair and also check status:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11285
How was this patch tested?