[ 
https://issues.apache.org/jira/browse/IGNITE-28870?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oleg Valuyskiy updated IGNITE-28870:
------------------------------------
    Summary: Restore snapshot temporary files cleanup logic  (was: False ERROR 
is logged during snapshot temporary directory cleanup)

> Restore snapshot temporary files cleanup logic
> ----------------------------------------------
>
>                 Key: IGNITE-28870
>                 URL: https://issues.apache.org/jira/browse/IGNITE-28870
>             Project: Ignite
>          Issue Type: Bug
>            Reporter: Oleg Valuyskiy
>            Assignee: Oleg Valuyskiy
>            Priority: Minor
>              Labels: ise
>         Attachments: SnapshotTmpDirCleanupReproducer.patch
>
>
> After the changes introduced in IGNITE-24130 / PR #11896, Ignite may log the 
> following ERROR after a snapshot is successfully created:
> {code:java}
> Snapshot directory doesn't exist [snpName=<snapshot_name>, 
> dir=<work_dir>/db/<node_storage>/snp]{code}
> The snapshot itself may be successfully created in the configured snapshot 
> directory. The error is logged during cleanup of temporary snapshot files.
> Reproducer: [^SnapshotTmpDirCleanupReproducer.patch]
> h3. Original cleanup logic
> Before IGNITE-24130 / PR #11896, snapshot cleanup used two different 
> temporary directories ({*}SnapshotFutureTask#onDone{*}):
> {code:java}
> U.delete(sft.tempFileTree().nodeStorage());
> // Delete snapshot directory if no other files exists.
> try {
>     if (U.fileCount(sft.tempFileTree().root().toPath()) == 0 || err != null)
>         U.delete(sft.tempFileTree().root().toPath());
> }
> catch (IOException e) {
>     log.error("Snapshot directory doesn't exist [snpName=" + snpName + ", 
> dir=" + sft.tempFileTree().root() + ']');
> }
> {code}
> These two directories represent different filesystem levels:
>  * sft.tempFileTree().root() = 
> <work_dir>/db/<node_storage>/snp/<snapshot_name>
>  * sft.tempFileTree().nodeStorage() = 
> <work_dir>/db/<node_storage>/snp/<snapshot_name>/db/<node_storage>
> So the original logic was:
>  * remove the node-specific temporary snapshot storage directory;
>  * then check the temporary root directory of the current snapshot;
>  * if the root directory has no other files left, remove it as well;
>  * if snapshot processing finished with an error, remove the temporary root 
> directory regardless of its contents.
> h3. Regression after IGNITE-24130 / PR #11896
> After the data region storage path support changes, the cleanup logic was 
> refactored into a helper method:
> {code:java}
> public static void removeTmpSnapshotFiles(SnapshotFileTree sft, boolean err, 
> IgniteLogger log) {
>     NodeFileTree tmpFt = sft.tempFileTree();
>     removeTmpDir(tmpFt.root(), err, log);
>     for (File tmpDrStorage : tmpFt.extraStorages().values())
>         removeTmpDir(tmpDrStorage.getParentFile(), err, log);
> }
> {code}
> The helper currently receives only one directory:
> {code:java}
> private static void removeTmpDir(File dir, boolean err, IgniteLogger log) {
>     U.delete(dir);
>     // Delete snapshot directory if no other files exists.
>     try {
>         if (U.fileCount(dir.toPath()) == 0 || err)
>             U.delete(dir.toPath());
>     }
>     catch (IOException e) {
>         log.error("Snapshot directory doesn't exist [snpName=" + 
> dir.getName() + ", dir=" + dir.getParentFile() + ']');
>     }
> }
> {code}
> This accidentally collapses the original two-level cleanup logic into a 
> single directory. The method now:
>  * removes the snapshot temporary root directory;
>  * then tries to count files in that same directory;
>  * then tries to delete the same directory again.
> So after:
> {code:java}
> U.delete(dir);{code}
> the following call may fail with NoSuchFileException:
> {code:java}
> U.fileCount(dir.toPath()){code}
> This results in a false ERROR in the log even though the snapshot temporary 
> directory has already been successfully removed.
> h3. Expected behavior
> The cleanup logic should preserve the original two-level semantics:
>  * remove the node-specific temporary snapshot storage directory;
>  * check the snapshot temporary root directory;
>  * remove the root directory if it becomes empty;
>  * remove the root directory if snapshot processing finished with an error;
>  * do not call U.fileCount(...) on the directory that has just been deleted;
>  * do not log an ERROR when the directory has already been removed 
> successfully.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to