SaketaChalamchala commented on code in PR #10387:
URL: https://github.com/apache/ozone/pull/10387#discussion_r3583231678
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java:
##########
@@ -392,25 +392,135 @@ private void init(OzoneConfiguration configuration,
SnapshotChainManager chainMa
}
- private void checkOrphanSnapshotVersions(OMMetadataManager metadataManager,
SnapshotChainManager chainManager)
+ /**
+ * Drains the snapshots currently queued in {@link
#snapshotToBeCheckedForOrphans}.
+ * Each queued snapshot becomes a seed for a same-run ancestor cascade.
+ */
+ private void checkQueuedOrphanSnapshotVersions(OMMetadataManager
metadataManager,
+ SnapshotChainManager chainManager)
throws IOException {
- for (Map.Entry<UUID, Integer> entry :
snapshotToBeCheckedForOrphans.entrySet()) {
- UUID snapshotId = entry.getKey();
- int countBeforeCheck = entry.getValue();
- checkOrphanSnapshotVersions(metadataManager, chainManager, snapshotId);
- decrementOrphanCheckCount(snapshotId, countBeforeCheck);
+ Set<UUID> processedSnapshotIds = new HashSet<>();
+ List<UUID> queuedSnapshotIds =
getQueuedSnapshotIdsByCreationTimeDesc(metadataManager, chainManager);
+ if (queuedSnapshotIds.isEmpty()) {
+ return;
+ }
+ LOG.info("Draining orphan snapshot cleanup queue with {} seed snapshots",
+ queuedSnapshotIds.size());
+ for (UUID snapshotId : queuedSnapshotIds) {
+ cascadeOrphanSnapshotChecksFrom(metadataManager, chainManager,
snapshotId, processedSnapshotIds);
}
+ LOG.debug("Finished orphan snapshot cleanup drain; {} snapshots remain
queued for future runs",
+ snapshotToBeCheckedForOrphans.size());
}
- @VisibleForTesting
- void checkOrphanSnapshotVersions(OMMetadataManager metadataManager,
SnapshotChainManager chainManager,
- UUID snapshotId) throws IOException {
+ /**
+ * Sorts the currently queued snapshot ids from newest to oldest using
snapshot creation
+ * time when that metadata is available. This is only a seed-order
optimization for the
+ * outer batch drain; the inner cascade still handles ancestors discovered
during the run.
+ */
+ private List<UUID> getQueuedSnapshotIdsByCreationTimeDesc(OMMetadataManager
metadataManager,
+ SnapshotChainManager chainManager) throws IOException {
+ List<UUID> snapshotIds = new
ArrayList<>(snapshotToBeCheckedForOrphans.keySet());
+ if (chainManager == null) {
+ LOG.debug("Snapshot chain manager unavailable; using queued orphan
snapshot iteration order for {} snapshots",
+ snapshotIds.size());
+ return snapshotIds;
+ }
+ Table<String, SnapshotInfo> snapshotInfoTable =
metadataManager.getSnapshotInfoTable();
+ if (snapshotInfoTable == null) {
+ LOG.debug("Snapshot info table unavailable; using queued orphan snapshot
iteration order for {} snapshots",
+ snapshotIds.size());
+ return snapshotIds;
+ }
+
+ Map<UUID, Long> creationTimeBySnapshotId = new HashMap<>();
+ for (UUID snapshotId : snapshotIds) {
+ String tableKey = chainManager.getTableKey(snapshotId);
+ if (tableKey == null) {
+ LOG.debug("Snapshot {} is queued for orphan cleanup but has no
snapshot info table key; "
+ + "leaving it in fallback iteration order", snapshotId);
+ continue;
+ }
+ SnapshotInfo snapshotInfo = snapshotInfoTable.get(tableKey);
+ if (snapshotInfo != null) {
+ creationTimeBySnapshotId.put(snapshotId,
snapshotInfo.getCreationTime());
+ } else {
+ LOG.debug("Snapshot {} is queued for orphan cleanup but snapshot info
is missing for table key {};"
+ + " leaving it in fallback iteration order", snapshotId,
tableKey);
+ }
+ }
+
+ // Prefer newer queued snapshots as seeds so descendants already waiting
in the batch
+ // are more likely to run before their ancestors.
+ snapshotIds.sort(Comparator.comparingLong(
+ snapshotId -> creationTimeBySnapshotId.getOrDefault(snapshotId,
Long.MIN_VALUE)).reversed());
+ LOG.debug("Ordered {} queued orphan snapshot seeds by creation time using
metadata for {} snapshots",
+ snapshotIds.size(), creationTimeBySnapshotId.size());
+ return snapshotIds;
+ }
+
+ /**
+ * Runs orphan cleanup starting from {@code snapshotId} and cascades toward
older snapshots
+ * in the same call whenever removing data from the current snapshot may
have unblocked its
+ * {@code previousSnapshotId}.
+ *
+ * <p>Callers that want an isolated one-off check should pass a fresh empty
+ * {@code processedSnapshotIds} set. {@link
#checkQueuedOrphanSnapshotVersions(
+ * OMMetadataManager, SnapshotChainManager)} passes a shared set for the
whole scheduler drain
+ * so a snapshot re-queued for retry is deferred to the next run instead of
being checked twice
+ * in the same batch.
+ *
+ * <p>For example, if the chain is {@code S3 -> S2 -> S1} and checking
{@code S3} removes
+ * orphan versions that make {@code S2} eligible, a single call seeded with
{@code S3}
+ * will check {@code S3}, then {@code S2}, then {@code S1} in that order.
+ */
+ void cascadeOrphanSnapshotChecksFrom(OMMetadataManager metadataManager,
+ SnapshotChainManager chainManager, UUID snapshotId, Set<UUID>
processedSnapshotIds)
+ throws IOException {
+ for (UUID currentSnapshotId = snapshotId; currentSnapshotId != null;) {
+ // A descendant processed earlier in the same drain may already have
checked
+ // this snapshot as an ancestor. If it re-queued itself for retry, leave
that
+ // retry for the next scheduler iteration instead of checking it twice
now.
+ if (!processedSnapshotIds.add(currentSnapshotId)) {
+ LOG.debug("Skipping snapshot {} because it was already processed
earlier in this orphan cleanup pass",
+ currentSnapshotId);
+ return;
Review Comment:
Consider this scenario:
1. this is the snapshot chain S3 -> S2 -> prev_chain and the dependency is
S3.v1 -> S2.v1,
S2.v1 -> prev_chain.v
2. S2 is purged but YAML not deleted, prev_chain is changed and defrag runs.
Dependency chain is
S3.v1 -> S2.v1,
S2.v1 -> prev_chain.v, S2.v2 -> prev_chain.v'
3. S4 is created and defragged and purged before YAMLs is cleaned.
Dependency chain is
S3.v1 -> S2.v1,
S2.v1 -> prev_chain.v, S2.v2 -> prev_chain.v'
S4.v1 -> S2.v2
The cleanup queue is now {S4, S3, S2, ...} if ordered by descending order of
snapshot creation.
When processing S4, S4 yaml is removed and the cascade reaches S2 and
removes S2.v2. In this case, S2 is marked as processed.
When S3 is processed and removes it's YAML. In the same run when cascading
S2.v1(and other prev_chain versions) is eligible for removal but, the cleanup
skips it and returns.
Would tracking processed versions for each snapshot be a better optimization?
--
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]
