-
Notifications
You must be signed in to change notification settings - Fork 592
HDDS-13188. Track total pending deletion size for FSO directories in OM via Recon #9252
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -642,6 +642,29 @@ private void getPendingForDeletionDirInfo( | |
| } | ||
| } | ||
|
|
||
| private void calculateTotalPendingDeletedDirSizes(Map<String, Long> dirSummary) { | ||
| long totalDataSize = 0L; | ||
| long totalReplicatedDataSize = 0L; | ||
|
|
||
| Table<String, OmKeyInfo> deletedDirTable = omMetadataManager.getDeletedDirTable(); | ||
| try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> iterator = deletedDirTable.iterator()) { | ||
| while (iterator.hasNext()) { | ||
| Table.KeyValue<String, OmKeyInfo> kv = iterator.next(); | ||
| OmKeyInfo omKeyInfo = kv.getValue(); | ||
| if (omKeyInfo != null) { | ||
| Pair<Long, Long> sizeInfo = fetchSizeForDeletedDirectory(omKeyInfo.getObjectID()); | ||
| totalDataSize += sizeInfo.getLeft(); | ||
| totalReplicatedDataSize += sizeInfo.getRight(); | ||
| } | ||
| } | ||
| } catch (IOException ex) { | ||
| throw new WebApplicationException(ex, Response.Status.INTERNAL_SERVER_ERROR); | ||
| } | ||
|
|
||
| dirSummary.put("totalDataSize", totalDataSize); | ||
| dirSummary.put("totalReplicatedDataSize", totalReplicatedDataSize); | ||
| } | ||
|
|
||
| /** | ||
| * Given an object ID, return total data size as a pair of Total Size, Total Replicated Size | ||
| * under this object. Note:- This method is RECURSIVE. | ||
|
|
@@ -744,6 +767,28 @@ public Response getDeletedDirectorySummary() { | |
| return Response.ok(dirSummary).build(); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves the summary of the total delete pending directory size (unreplicated and replicated). | ||
| * | ||
| * @return The HTTP response body includes a map with the following entries: | ||
| * - "totalDataSize": the total replicated size of delete pending directories. | ||
| * - "totalReplicatedDataSize": the total unreplicated size of delete pending directories. | ||
| * | ||
| * Example response: | ||
| * { | ||
| * "totalDataSize": 30000, | ||
| * "totalReplicatedDataSize": 90000 | ||
| * } | ||
| */ | ||
| @GET | ||
| @Path("/deletePending/dirs/size-summary") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How this API is different from existing one here ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @devmadhuu , this "/deletePending/dirs/size-summary" is a new API, returns calculated size and replicated size. "/deletePending/dirs/summary" is a existing API, returns count from DB directly without calculation. Do you think we should merge them into one, or keep it separately?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see that existing API and new API both are iterating
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyway we are using this api inside recon only. So I think instead of exposing as API, we can use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still not able to understand, why we need that new API code ? Existing API, very minor changes needed related to limit. Can you explain ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a summary API, so I guess limit doesn't apply. Ideally, we should add the new function in "/deletePending/dirs/summary" too, as the pending deletion key does, so all the APIs' behavior is consistent. The difference is for directory, it iterate the omMetadataManager.getDeletedDirTable() table to calculate all the sizes, for key, it does this I'm not very familiar with Recon code. So does above code just read the data from somewhere and doesn't require any further calculation? My concern regarding how we use "/deletePending/summary" and "/deletePending/dirs/summary" in Recon UI, if pending deletion directory size calculation is expensive, and in Recon UI, where only totalDeletedDirectories is required, if we calculate the size in each API call, the data is not used and wasted, in meanwhile also add the latency to this API call. I'm fine to merge the two API into one if that's not a problem.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can close this PR. Instead of maintaining two separate methods, we can simplify the logic by passing limit = -1 when we want to retrieve all results. Inside the loop, we can add a check like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ChenSammi This PR is adding an API for deletePending directories size summary. These all existing APIs / |
||
| public Response getTotalDeletedDirectorySizeSummary() { | ||
| Map<String, Long> dirSummary = new HashMap<>(); | ||
| // Create a keys summary for deleted directories | ||
| calculateTotalPendingDeletedDirSizes(dirSummary); | ||
| return Response.ok(dirSummary).build(); | ||
| } | ||
|
|
||
| /** | ||
| * This API will list out limited 'count' number of keys after applying below filters in API parameters: | ||
| * Default Values of API param filters: | ||
|
|
||
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.
Please log a message for this ex to know why it fails.