Copilot commented on code in PR #9810:
URL: https://github.com/apache/ozone/pull/9810#discussion_r2842414450
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java:
##########
@@ -378,7 +393,9 @@ public synchronized boolean deleteSnapshot(SnapshotInfo
snapshotInfo)
*/
public synchronized void removeFromSnapshotIdToTable(UUID snapshotId) throws
IOException {
validateSnapshotChain();
- snapshotIdToTableKey.remove(snapshotId);
+ String tableKey = snapshotIdToTableKey.remove(snapshotId);
+ LOG.debug("Removed from snapshotIdToTableKey: snapshotId={} tableKey={}.
caller: {}",
+ snapshotId, tableKey, Thread.currentThread().getStackTrace()[2]);
Review Comment:
The stack trace access using `Thread.currentThread().getStackTrace()[2]` is
fragile and may break if the call stack changes. Additionally, capturing stack
traces for debugging can have performance implications. Consider using a
simpler approach like passing a caller context string as a parameter, or
removing the caller information from the log message if it's not critical for
debugging.
```suggestion
LOG.debug("Removed from snapshotIdToTableKey: snapshotId={} tableKey={}",
snapshotId, tableKey);
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java:
##########
@@ -353,6 +357,17 @@ public synchronized void addSnapshot(SnapshotInfo
snapshotInfo)
// store snapshot ID to snapshot DB table key in the map
snapshotIdToTableKey.put(snapshotInfo.getSnapshotId(),
snapshotInfo.getTableKey());
+ LOG.debug("Added to snapshotIdToTableKey: snapshotId={} tableKey={}",
+ snapshotInfo.getSnapshotId(), snapshotInfo.getTableKey());
+ }
+
+ /**
+ * Used by OMSnapshotCreateResponse#addToDBBatch to add to
snapshotIdToTableKey map during Raft replay.
Review Comment:
The comment states this method is "Used by
OMSnapshotCreateResponse#addToDBBatch" but doesn't explain why it's needed or
the broader context. Consider enhancing the documentation to explain that this
method is specifically needed during Raft log replay, when addToDBBatch is
called but validateAndUpdateCache is skipped, to ensure the
snapshotIdToTableKey map stays synchronized with the database state.
```suggestion
* Maintain the {@code snapshotIdToTableKey} map during Raft log replay.
* <p>
* In the normal (non-replay) execution path, {@link
#addSnapshot(SnapshotInfo)}
* is called via {@code validateAndUpdateCache}, which both updates the
* on-disk SnapshotInfo table and keeps the in-memory
* {@code snapshotIdToTableKey} map in sync.
* <p>
* During Raft log replay, however, {@code
OMSnapshotCreateResponse#addToDBBatch}
* is invoked to rebuild the SnapshotInfo table state, but
* {@code validateAndUpdateCache} is intentionally skipped. In that case
this
* method must be called so that {@code snapshotIdToTableKey} is updated to
* reflect the snapshot entries being written to the database, ensuring the
* in-memory map stays consistent with the persisted SnapshotInfo table.
*
* @param snapshotId snapshot identifier whose table key is being recorded
* @param tableKey key used in the SnapshotInfo table for this snapshot
```
--
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]
