devmadhuu commented on code in PR #9127:
URL: https://github.com/apache/ozone/pull/9127#discussion_r2434686818
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -185,12 +188,14 @@ protected void handleDeleteKeyEvent(OmKeyInfo keyInfo,
// Decrement immediate parent's totals (these fields now represent totals)
nsSummary.setNumOfFiles(nsSummary.getNumOfFiles() - 1);
nsSummary.setSizeOfFiles(nsSummary.getSizeOfFiles() -
keyInfo.getDataSize());
+ nsSummary.setReplicatedSizeOfFiles(nsSummary.getReplicatedSizeOfFiles() -
keyInfo.getReplicatedSize());
Review Comment:
```suggestion
long currentReplSize = nsSummary.getReplicatedSizeOfFiles();
long keyReplSize = keyInfo.getReplicatedSize();
if (currentReplSize >= 0 && keyReplSize >= 0) {
nsSummary.setReplicatedSizeOfFiles(currentReplSize - keyReplSize);
}
```
else this can corrupt the data. E.g.
This could be problematic! If:
- nsSummary.getReplicatedSizeOfFiles() returns -1 (old data)
- keyInfo.getReplicatedSize() returns, say, 100
- Result: -1 - 100 = -101
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -98,18 +98,19 @@ protected void handlePutKeyEvent(OmKeyInfo keyInfo,
Map<Long,
nsSummary = new NSSummary();
}
int[] fileBucket = nsSummary.getFileSizeBucket();
-
- // Update immediate parent's totals (these fields now represent totals)
+
+ // Update immediate parent's totals (includes all descendant files)
nsSummary.setNumOfFiles(nsSummary.getNumOfFiles() + 1);
nsSummary.setSizeOfFiles(nsSummary.getSizeOfFiles() +
keyInfo.getDataSize());
+ nsSummary.setReplicatedSizeOfFiles(nsSummary.getReplicatedSizeOfFiles() +
keyInfo.getReplicatedSize());
Review Comment:
```suggestion
// Before arithmetic operations, check for sentinel value
long currentReplSize = nsSummary.getReplicatedSizeOfFiles();
if (currentReplSize < 0) {
// Old data, initialize to 0 before first use
currentReplSize = 0;
nsSummary.setReplicatedSizeOfFiles(0);
}
nsSummary.setReplicatedSizeOfFiles(currentReplSize +
keyInfo.getReplicatedSize());
```
Check similar data corruption issues else where in code.
--
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]
