prashantpogde commented on code in PR #7112:
URL: https://github.com/apache/ozone/pull/7112#discussion_r1731933291
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -674,19 +675,36 @@ private ReferenceCounted<OmSnapshot> getSnapshot(String
snapshotTableKey, boolea
}
/**
- * Returns true if the snapshot is in given status.
- * @param key DB snapshot table key
- * @param status SnapshotStatus
- * @return true if the snapshot is in given status, false otherwise
+ * Returns OmSnapshot object and skips active check.
+ * This should only be used for API calls initiated by background service
e.g. purgeKeys, purgeSnapshot,
+ * snapshotMoveDeletedKeys, and SetSnapshotProperty.
*/
- public boolean isSnapshotStatus(String key,
- SnapshotInfo.SnapshotStatus status)
- throws IOException {
- return getSnapshotInfo(key).getSnapshotStatus().equals(status);
+ public ReferenceCounted<OmSnapshot> getSnapshot(UUID snapshotId) throws
IOException {
+ return snapshotCache.get(snapshotId);
}
- public SnapshotInfo getSnapshotInfo(String key) throws IOException {
- return SnapshotUtils.getSnapshotInfo(ozoneManager, key);
+ /**
+ * Returns snapshotInfo from cache if it is present in cache, otherwise it
checks RocksDB and return value from there.
+ * This should be used by SnapshotCache only.
+ * Sometimes, the follower OM node may be lagging that it gets purgeKeys or
snapshotMoveDeletedKeys from a Snapshot,
+ * and purgeSnapshot for the same Snapshot one after another. And
purgeSnapshot's validateAndUpdateCache gets
+ * executed before doubleBuffer flushes purgeKeys or snapshotMoveDeletedKeys
from that Snapshot.
+ * This should not be a case on the leader node because
SnapshotDeletingService checks that deletedTable and
+ * deletedDirectoryTable in DB don't have entries for the bucket before it
sends a purgeSnapshot on a snapshot.
+ * If that happens, and we just look into the cache, the addToBatch
operation will fail when it tries to open
+ * the DB and purgeKeys from the Snapshot because snapshot is already purged
from the SnapshotInfoTable cache.
+ * Hence, it is needed to look into the table to make sure that snapshot
exists somewhere either in cache or in DB.
+ */
+ private SnapshotInfo getSnapshotInfo(String snapshotKey) throws IOException {
+ SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable().get(snapshotKey);
+
+ if (snapshotInfo == null) {
+ snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable().getSkipCache(snapshotKey);
Review Comment:
The downside is, it can mask other hidden bugs that were accepting snapshot
to be in cache. Perhaps we can log it here with stack trace.
--
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]
