devmadhuu commented on code in PR #5037:
URL: https://github.com/apache/ozone/pull/5037#discussion_r1258421449
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -271,14 +327,15 @@ private void handlePutEvent(OMDBUpdateEvent<String,
Object> event,
Collection<String> sizeRelatedTables,
Review Comment:
We don't need to pass this argument as "getTablesToCalculateSize" is public
method.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -179,6 +200,40 @@ private Triple<Long, Long, Long> getTableSizeAndCount(
return Triple.of(count, unReplicatedSize, replicatedSize);
}
+ /**
+ * Fetches the size of a deleted directory identified by the given path.
+ * The size is obtained from the NSSummary table using the directory
objectID.
+ * The path is expected to be in the format :-
+ * "volumeId/bucketId/parentId/dirName/dirObjectId".
+ *
+ * @param path The path of the deleted directory.
+ * @return The size of the deleted directory.
+ * @throws IOException If an I/O error occurs while retrieving the size.
+ */
+ public long fetchSizeForDeletedDirectory(String path)
+ throws IOException {
+ if (path == null || path.isEmpty()) {
+ return 0L;
+ }
+ String[] parts = path.split("/");
+ String directoryObjectId = parts.length >= 6 ? parts[5] : "";
Review Comment:
Can you add a comment why this special figure of 6 for length check ?
Because as per key example, dirObjectId is at 4th index itself ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -337,21 +407,34 @@ private void handleDeleteEvent(OMDBUpdateEvent<String,
Object> event,
private void handleSizeRelatedTableDeleteEvent(
OMDBUpdateEvent<String, Object> event, String countKey,
- String unReplicatedSizeKey, String replicatedSizeKey,
+ String unReplicatedSizeKey, String replicatedSizeKey, String tableName,
HashMap<String, Long> objectCountMap,
HashMap<String, Long> unreplicatedSizeCountMap,
- HashMap<String, Long> replicatedSizeCountMap) {
+ HashMap<String, Long> replicatedSizeCountMap) throws IOException {
+
+ Boolean isFileTable = tableName.equals(OPEN_FILE_TABLE);
+ Boolean isKeyTable = tableName.equals(OPEN_KEY_TABLE);
+ Boolean isDeletedDirTable = tableName.equals(DELETED_DIR_TABLE);
+
if (event.getValue() instanceof OmKeyInfo) {
// Handle DELETE for OpenKeyTable & OpenFileTable
OmKeyInfo omKeyInfo = (OmKeyInfo) event.getValue();
objectCountMap.computeIfPresent(countKey,
(k, count) -> count > 0 ? count - 1L : 0L);
- unreplicatedSizeCountMap.computeIfPresent(unReplicatedSizeKey,
- (k, size) -> size > omKeyInfo.getDataSize() ?
- size - omKeyInfo.getDataSize() : 0L);
- replicatedSizeCountMap.computeIfPresent(replicatedSizeKey,
- (k, size) -> size > omKeyInfo.getReplicatedSize() ?
- size - omKeyInfo.getReplicatedSize() : 0L);
+ if (isFileTable || isKeyTable) {
Review Comment:
Try to reduce if else complexity as this method can grow in future for file
and key tables also.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -287,18 +344,30 @@ private void handlePutEvent(OMDBUpdateEvent<String,
Object> event,
private void handleSizeRelatedTablePutEvent(
OMDBUpdateEvent<String, Object> event, String countKey,
- String unReplicatedSizeKey, String replicatedSizeKey,
+ String unReplicatedSizeKey, String replicatedSizeKey, String tableName,
HashMap<String, Long> objectCountMap,
HashMap<String, Long> unreplicatedSizeCountMap,
- HashMap<String, Long> replicatedSizeCountMap) {
+ HashMap<String, Long> replicatedSizeCountMap) throws IOException {
+
+ boolean isFileTable = tableName.equals(OPEN_FILE_TABLE);
+ boolean isKeyTable = tableName.equals(OPEN_KEY_TABLE);
+ boolean isDeletedDirTable = tableName.equals(DELETED_DIR_TABLE);
+
// Handle PUT for OpenKeyTable & OpenFileTable
if (event.getValue() instanceof OmKeyInfo) {
OmKeyInfo omKeyInfo = (OmKeyInfo) event.getValue();
objectCountMap.computeIfPresent(countKey, (k, count) -> count + 1L);
- unreplicatedSizeCountMap.computeIfPresent(unReplicatedSizeKey,
- (k, size) -> size + omKeyInfo.getDataSize());
- replicatedSizeCountMap.computeIfPresent(replicatedSizeKey,
- (k, size) -> size + omKeyInfo.getReplicatedSize());
+ if (isFileTable || isKeyTable) {
Review Comment:
We can reduce if else complexity here, as we know that size of tables are
meant based on files/keys which can be computed later at bucket or volume level
as well or can be computed for file and key table as well , try to keep the
code generic as much as possible.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -287,18 +344,30 @@ private void handlePutEvent(OMDBUpdateEvent<String,
Object> event,
private void handleSizeRelatedTablePutEvent(
OMDBUpdateEvent<String, Object> event, String countKey,
- String unReplicatedSizeKey, String replicatedSizeKey,
+ String unReplicatedSizeKey, String replicatedSizeKey, String tableName,
HashMap<String, Long> objectCountMap,
HashMap<String, Long> unreplicatedSizeCountMap,
- HashMap<String, Long> replicatedSizeCountMap) {
+ HashMap<String, Long> replicatedSizeCountMap) throws IOException {
+
+ boolean isFileTable = tableName.equals(OPEN_FILE_TABLE);
Review Comment:
variable name is confusing as these are for openfile and openkey tables,
though these might not be needed if we can control at the caller of this method
itself.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -139,28 +150,38 @@ public Pair<String, Boolean> reprocess(OMMetadataManager
omMetadataManager) {
* Returns a triple with the total count of records (left), total
unreplicated
* size (middle), and total replicated size (right) in the given iterator.
* Increments count for each record and adds the dataSize if a record's value
- * is an instance of OmKeyInfo. If the iterator is null, returns (0, 0, 0).
+ * is an instance of OmKeyInfo,RepeatedOmKeyInfo.
+ * If the iterator is null, returns (0, 0, 0).
*
* @param iterator The iterator over the table to be iterated.
* @return A Triple with three Long values representing the count,
* unreplicated size and replicated size.
* @throws IOException If an I/O error occurs during the iterator traversal.
*/
private Triple<Long, Long, Long> getTableSizeAndCount(
- TableIterator<String, ? extends Table.KeyValue<String, ?>> iterator)
+ TableIterator<String, ? extends Table.KeyValue<String, ?>> iterator,
Review Comment:
@ArafatKhan2198 thanks for working on this patch, can you pls elaborate the
purpose of this method since you added tableName as extra parameter now. Is
this method only for counting records of tables which are having keys as
OMKeyInfo and RepeatedOmKeyInfo only ?. Also if GlobalStatsTable will not have
counts of OpenFile and OpenKey table ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
