Copilot commented on code in PR #9313:
URL: https://github.com/apache/ozone/pull/9313#discussion_r2545976371
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalData.java:
##########
@@ -67,6 +67,9 @@ public class OmSnapshotLocalData implements
WithChecksum<OmSnapshotLocalData> {
// Stores the transactionInfo corresponding to OM when the snaphot is purged.
private TransactionInfo transactionInfo;
+ // Stores the rocksDB's transaction sequence number at the time of snapshot
creation.'
Review Comment:
Typo in comment: trailing apostrophe should be removed. The period at the
end is sufficient.
```suggestion
// Stores the rocksDB's transaction sequence number at the time of
snapshot creation.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java:
##########
@@ -172,9 +172,10 @@ public Object construct(Node node) {
final String prevSnapIdStr = (String)
nodes.get(OzoneConsts.OM_SLD_PREV_SNAP_ID);
UUID prevSnapId = prevSnapIdStr != null ?
UUID.fromString(prevSnapIdStr) : null;
final String purgeTxInfoStr = (String)
nodes.get(OzoneConsts.OM_SLD_TXN_INFO);
+ final long dbTxnSeqNumber =
((Number)nodes.get(OzoneConsts.OM_SLD_DB_TXN_SEQ_NUMBER)).longValue();
Review Comment:
Potential NullPointerException when reading existing YAML files that don't
have the 'dbTxSequenceNumber' field. The code assumes the field exists and
directly calls `.longValue()` without null-checking. Consider using
`getOrDefault()` with a default value of 0L, similar to how 'lastDefragTime' is
handled on lines 188-195.
```suggestion
Object dbTxnSeqNumberObj =
nodes.getOrDefault(OzoneConsts.OM_SLD_DB_TXN_SEQ_NUMBER, 0L);
long dbTxnSeqNumber;
if (dbTxnSeqNumberObj instanceof Number) {
dbTxnSeqNumber = ((Number) dbTxnSeqNumberObj).longValue();
} else {
throw new IllegalArgumentException("Invalid type for
dbTxSequenceNumber: " +
dbTxnSeqNumberObj.getClass().getName() + ". Expected Number
type.");
}
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/diff/delta/TestRDBDifferComputer.java:
##########
@@ -363,7 +363,7 @@ public void testComputeDeltaFilesWithVersionMapping()
throws IOException {
* Tests that getDSIFromSI throws exception when no versions found.
Review Comment:
The comment still references the old method name 'getDSIFromSI'. It should
be updated to 'toDifferSnapshotInfo' to match the renamed method on line 366.
```suggestion
* Tests that toDifferSnapshotInfo throws exception when no versions found.
```
--
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]
