zstan commented on code in PR #13577:
URL: https://github.com/apache/ignite/pull/13577#discussion_r4142386190


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -701,46 +708,105 @@ public IgniteSnapshotManager(GridKernalContext ctx) {
     /**
      * @param snpDir Snapshot dir.
      */
-    public void deleteSnapshot(File snpDir) {
+    public void deleteLocalSnapshot(File snpDir) {
         if (!snpDir.exists())
             return;
 
         if (!snpDir.isDirectory())
             return;
 
-        deleteSnapshot(new SnapshotFileTree(
+        var sft = new SnapshotFileTree(
             cctx.kernalContext(),
             snpDir.getName(),
             snpDir.getParent(),
             ft.folderName(),
-            pdsSettings.consistentId().toString()));
+            pdsSettings.consistentId().toString()
+        );
+
+        deleteLocalSnapshot(sft);
     }
 
-    /** */
-    public void deleteSnapshot(SnapshotFileTree sft) {
-        try {
-            U.delete(sft.binaryMeta());
-            sft.allStorages().forEach(U::delete);
-            U.delete(sft.meta());
+    /**
+     * Tries to delete local snapshot data.
+     *
+     * @param sft Snapshot file tree.
+     * @return A pair of {@code boolean} values. The first indicates whether 
snapshot was completely deleted. The second
+     *         indicates whether the snapshot was found at all.
+     */
+    public T2<Boolean, Boolean> deleteLocalSnapshot(SnapshotFileTree sft) {
+        T2<Boolean, Boolean> res = new T2<>(false, false);
+
+        if (sft.root().exists())
+            res.set2(true);
+        else {
+            for (File storage : sft.allStorages().toList()) {
+                if (storage.exists())
+                    res.set2(true);
+            }
+        }
+
+        // Not found at all - nothing to delete.
+        if (!res.get2())
+            return res;
+
+        // Assume we'll successed.
+        res.set1(true);
 
-            deleteDirectory(sft.binaryMetaRoot());
-            deleteDirectory(sft.marshaller());
+        // The 'exists' checks are for a concurrent deletion when nodes share 
their working and snapshot directories.
+        // Nodes may steal removal jobs and the files aren't synchronized. 
There are gaps between and `exists()` and `delete()`.
+        // We try to delete first. If snapshot data wasn't deleted because it 
doesn't already exist is not a removal error here.
+        try {
+            if (!sft.meta().delete() && sft.meta().exists())
+                res.set1(false);
+
+            for (var s : sft.allStorages().toList()) {

Review Comment:
   var - inacceptable



-- 
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]

Reply via email to