From c994153ee4d0cfd529eca52130cdbaa00c4e92b4 Mon Sep 17 00:00:00 2001 From: Christofer Dutz Date: Wed, 11 Nov 2026 13:25:23 +0100 Subject: [PATCH 1/2] [Storage] Make three TierManager fields thread-safe for SonarCloud's reliability gate SonarCloud reports java:S3077 for three volatile fields in TierManager that hold a mutable map, an array and a mutable FolderManager. They are now AtomicReferences: folderManagers holds an unmodifiable map that rebuildFolders() replaces as a whole, tierDiskTotalSpace an array that is replaced as a whole and copied once in getTierDiskTotalSpace(), and TierFolderManager.manager the FolderManager created by the memoized supplier. The identity check in hasSpaceForRecovery() still compares the map read before the check with the one published after it. Co-Authored-By: Claude Opus 5.5 --- .../rescon/disk/TierManager.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManager.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManager.java index 1a51b78c452b..f441894a9644 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManager.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManager.java @@ -46,6 +46,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -53,6 +54,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; /** The main class of multiple directories. Used to allocate folders to data files. */ @@ -62,7 +64,10 @@ public class TierManager { private record TierDirectory(int tierLevel, String folderName) {} - private volatile Map folderManagers = Map.of(); + /** Replaced as a whole by {@link #rebuildFolders()}; the published maps are unmodifiable. */ + private final AtomicReference> folderManagers = + new AtomicReference<>(Map.of()); + // A failed constructor can report DiskFull before the replacement managers are published. private volatile boolean rebuildingFolders; @@ -75,7 +80,7 @@ private record TierDirectory(int tierLevel, String folderName) {} private List objectDirs; /** total space of each tier, Long.MAX_VALUE when one tier contains remote storage */ - private volatile long[] tierDiskTotalSpace = new long[0]; + private final AtomicReference tierDiskTotalSpace = new AtomicReference<>(new long[0]); private TierManager() { initFolders(); @@ -206,10 +211,10 @@ private void rebuildFolders() { } } - tierDiskTotalSpace = getTierDiskSpace(DiskSpaceType.TOTAL); + tierDiskTotalSpace.set(getTierDiskSpace(DiskSpaceType.TOTAL)); seqDir2TierLevel = newSeqDir2TierLevel; unSeqDir2TierLevel = newUnSeqDir2TierLevel; - folderManagers = newManagers; + folderManagers.set(Collections.unmodifiableMap(newManagers)); } private void addFolderManager( @@ -233,20 +238,22 @@ private void addFolderManager( private final class TierFolderManager { private final List folders; private final MemoizedCheckedSupplier supplier; - private volatile FolderManager manager; + private final AtomicReference manager = new AtomicReference<>(); private TierFolderManager(TierDirectory key, List folders, DirectoryStrategyType type) { this.folders = folders; supplier = MemoizedCheckedSupplier.valueOf( () -> { - manager = new FolderManager(folders, type, () -> reportDiskFull(key, this)); - return manager; + FolderManager created = + new FolderManager(folders, type, () -> reportDiskFull(key, this)); + manager.set(created); + return created; }); } private boolean hasSpaceForRecovery() { - FolderManager current = manager; + FolderManager current = manager.get(); return current == null ? folders.stream().anyMatch(JVMCommonUtils::hasSpaceWithoutRetry) : current.hasSpaceForRecovery(); @@ -254,7 +261,7 @@ private boolean hasSpaceForRecovery() { } private void reportDiskFull(TierDirectory key, TierFolderManager source) { - if (!rebuildingFolders && folderManagers.get(key) != source) { + if (!rebuildingFolders && folderManagers.get().get(key) != source) { return; } CommonDescriptor.getInstance() @@ -263,13 +270,13 @@ private void reportDiskFull(TierDirectory key, TierFolderManager source) { } private boolean hasSpaceForRecovery(TierDirectory key) { - Map active = folderManagers; + Map active = folderManagers.get(); if (rebuildingFolders) { return false; } TierFolderManager current = active.get(key); boolean recovered = current == null || current.hasSpaceForRecovery(); - return recovered && active == folderManagers && !rebuildingFolders; + return recovered && active == folderManagers.get() && !rebuildingFolders; } public synchronized void resetFolders() { @@ -319,7 +326,7 @@ public FolderManager getFolderManager(int tierLevel, boolean sequence) private FolderManager getFolderManager(int tierLevel, String folderName) throws DiskSpaceInsufficientException { TierDirectory key = new TierDirectory(tierLevel, folderName); - TierFolderManager entry = folderManagers.get(key); + TierFolderManager entry = folderManagers.get().get(key); if (entry == null) { throw new IndexOutOfBoundsException(key.toString()); } @@ -414,7 +421,7 @@ public List getAllMatchedObjectDirs(String regionIdStr, String... path) { } public int getTiersNum() { - return tierDiskTotalSpace.length; + return tierDiskTotalSpace.get().length; } public int getFileTierLevel(File file) { @@ -445,7 +452,7 @@ public int getFileTierLevel(File file) { } public long[] getTierDiskTotalSpace() { - return Arrays.copyOf(tierDiskTotalSpace, tierDiskTotalSpace.length); + return tierDiskTotalSpace.get().clone(); } public long[] getTierDiskUsableSpace() { From 39f8a7bd22bd6c77e18cbe16f1d80b6091465f70 Mon Sep 17 00:00:00 2001 From: Christofer Dutz Date: Wed, 11 Nov 2026 14:12:46 +0100 Subject: [PATCH 2/2] [Storage] Test that the tier disk total space follows the tiers and is returned as a copy Co-Authored-By: Claude Opus 5.5 --- .../rescon/disk/TierManagerTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManagerTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManagerTest.java index defedd56b899..0adc5cdbb766 100644 --- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManagerTest.java +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/rescon/disk/TierManagerTest.java @@ -84,6 +84,24 @@ public void tearDown() { tierManager.resetFolders(); } + @Test + public void testTierDiskTotalSpaceFollowsTheTiersAndIsCopied() { + config.setTierDataDirs( + new String[][] { + {new File(temporaryFolder.getRoot(), "tier0").getAbsolutePath()}, + {new File(temporaryFolder.getRoot(), "tier1").getAbsolutePath()} + }); + tierManager.resetFolders(); + + long[] totalSpace = tierManager.getTierDiskTotalSpace(); + assertEquals(2, totalSpace.length); + assertEquals(tierManager.getTiersNum(), totalSpace.length); + assertTrue(totalSpace[0] > 0 && totalSpace[1] > 0); + + totalSpace[0] = -1; + assertTrue(tierManager.getTierDiskTotalSpace()[0] > 0); + } + @Test public void testConcurrentResetFoldersAndGetNextFolder() throws Exception { List> folderAccessors =