Skip to content

Commit 9cd6644

Browse files
hongzhi-gaohongzhi-gao
authored
fix: Failed to remove unwritable directories when creating tsfile (apache#16122)
* add FolderManagerTest * fix: auto remove inaccessible folder * fix: auto remove inaccessible folder * mvn spotless:apply * Mitigate race condition risks: Directory creation by concurrent threads/processes may occur between the exists() verification and mkdirs() execution
1 parent 256f3e7 commit 9cd6644

File tree

2 files changed

+197
-1
lines changed

2 files changed

+197
-1
lines changed

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/tsfile/generator/TsFileNameGenerator.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,15 @@ public static String generateNewTsFilePathWithMkdir(
107107
+ virtualStorageGroup
108108
+ File.separator
109109
+ timePartitionId;
110-
fsFactory.getFile(tsFileDir).mkdirs();
110+
File targetDir = fsFactory.getFile(tsFileDir);
111+
if (!targetDir.exists()) {
112+
if (!targetDir.mkdirs() && !targetDir.exists()) {
113+
throw new IOException(
114+
"Directory creation failed: "
115+
+ tsFileDir
116+
+ " (Permission denied or parent not writable)");
117+
}
118+
}
111119
return tsFileDir
112120
+ File.separator
113121
+ generateNewTsFileName(

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/rescon/disk/FolderManagerTest.java‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.db.storageengine.rescon.disk;
21+
22+
import org.apache.iotdb.db.exception.DiskSpaceInsufficientException;
23+
import org.apache.iotdb.db.storageengine.rescon.disk.strategy.DirectoryStrategyType;
24+
25+
import org.apache.tsfile.fileSystem.FSFactoryProducer;
26+
import org.apache.tsfile.fileSystem.fsFactory.FSFactory;
27+
import org.junit.Before;
28+
import org.junit.Ignore;
29+
import org.junit.Rule;
30+
import org.junit.Test;
31+
import org.junit.rules.TemporaryFolder;
32+
import org.junit.runner.RunWith;
33+
import org.junit.runners.Parameterized;
34+
35+
import java.io.File;
36+
import java.io.IOException;
37+
import java.util.Arrays;
38+
import java.util.Collection;
39+
import java.util.List;
40+
41+
import static org.junit.Assert.assertNotEquals;
42+
import static org.junit.Assert.assertNotNull;
43+
import static org.junit.Assert.assertTrue;
44+
import static org.junit.Assert.fail;
45+
46+
@RunWith(Parameterized.class)
47+
public class FolderManagerTest {
48+
49+
@Rule public TemporaryFolder tempFolder = new TemporaryFolder();
50+
51+
private final DirectoryStrategyType strategyType;
52+
private FolderManager folderManager;
53+
private List<String> testFolders;
54+
private static FSFactory fsFactory = FSFactoryProducer.getFSFactory();
55+
56+
public FolderManagerTest(DirectoryStrategyType strategyType) {
57+
this.strategyType = strategyType;
58+
}
59+
60+
@Parameterized.Parameters
61+
public static Collection<DirectoryStrategyType> strategyTypes() {
62+
return Arrays.asList(DirectoryStrategyType.values());
63+
}
64+
65+
@Before
66+
public void setUp() throws Exception {
67+
File folder1 = tempFolder.newFolder("folder1");
68+
File folder2 = tempFolder.newFolder("folder2");
69+
File folder3 = tempFolder.newFolder("folder3");
70+
71+
testFolders =
72+
Arrays.asList(
73+
folder1.getAbsolutePath(), folder2.getAbsolutePath(), folder3.getAbsolutePath());
74+
75+
folderManager = new FolderManager(testFolders, strategyType);
76+
}
77+
78+
@Test
79+
public void testSuccessfulDirectoryCreation() throws Exception {
80+
String result =
81+
folderManager.getNextWithRetry(
82+
baseDir -> {
83+
String tsFileDir =
84+
baseDir
85+
+ File.separator
86+
+ "logicalSG"
87+
+ File.separator
88+
+ "virtualSG"
89+
+ File.separator
90+
+ "1";
91+
92+
File targetDir = fsFactory.getFile(tsFileDir);
93+
if (!(targetDir.exists() || targetDir.mkdirs())) {
94+
throw new IOException(tsFileDir + " directory creation failure");
95+
}
96+
97+
return tsFileDir + File.separator + "test.tsfile";
98+
});
99+
100+
assertNotNull(result);
101+
assertTrue(result.endsWith("test.tsfile"));
102+
assertTrue(new File(result).getParentFile().exists());
103+
}
104+
105+
@Test
106+
public void testRetryAfterFailure() throws Exception {
107+
// Array elements can be modified in lambdas despite final reference
108+
final boolean[] firstAttempt = {true};
109+
final String[] firstFolder = {null};
110+
111+
String result =
112+
folderManager.getNextWithRetry(
113+
baseDir -> {
114+
if (firstAttempt[0]) {
115+
firstAttempt[0] = false;
116+
firstFolder[0] = baseDir;
117+
throw new IOException("Simulated directory creation failure");
118+
}
119+
120+
// Verify we got a different folder on retry
121+
assertNotEquals(firstFolder[0], baseDir);
122+
123+
String tsFileDir =
124+
baseDir
125+
+ File.separator
126+
+ "logicalSG"
127+
+ File.separator
128+
+ "virtualSG"
129+
+ File.separator
130+
+ "1";
131+
132+
File targetDir = fsFactory.getFile(tsFileDir);
133+
if (!(targetDir.exists() || targetDir.mkdirs())) {
134+
throw new IOException(tsFileDir + " directory creation failure");
135+
}
136+
137+
return tsFileDir + File.separator + "retry.tsfile";
138+
});
139+
140+
assertNotNull(result);
141+
assertTrue(result.endsWith("retry.tsfile"));
142+
assertTrue(new File(result).getParentFile().exists());
143+
}
144+
145+
@Ignore("Test requires manual setup: directory must be set as immutable")
146+
@Test
147+
public void testImmutableBaseDir() throws Exception {
148+
// Requires manual setup: directory must be set as immutable
149+
String immutableFolder = testFolders.get(0);
150+
String result =
151+
folderManager.getNextWithRetry(
152+
baseDir -> {
153+
String tsFileDir =
154+
baseDir
155+
+ File.separator
156+
+ "logicalSG"
157+
+ File.separator
158+
+ "virtualSG"
159+
+ File.separator
160+
+ "1";
161+
File targetDir = fsFactory.getFile(tsFileDir);
162+
if (!(targetDir.exists() || targetDir.mkdirs())) {
163+
throw new IOException(tsFileDir + " directory creation failure");
164+
}
165+
return tsFileDir + File.separator + "immutable_test.tsfile";
166+
});
167+
168+
assertNotNull("Result path should not be null", result);
169+
assertTrue("File should end with correct suffix", result.endsWith("immutable_test.tsfile"));
170+
assertTrue("Parent directory should exist", new File(result).getParentFile().exists());
171+
}
172+
173+
@Test
174+
public void testEventuallyThrowsDiskFullAfterRetries() {
175+
try {
176+
folderManager.getNextWithRetry(
177+
baseDir -> {
178+
throw new IOException("Persistent failure");
179+
});
180+
fail("Expected DiskSpaceInsufficientException");
181+
} catch (DiskSpaceInsufficientException e) {
182+
// Expected after all retries fail
183+
assertTrue(e.getMessage().contains("Can't get next folder"));
184+
} catch (Exception e) {
185+
fail("Should have thrown DiskSpaceInsufficientException");
186+
}
187+
}
188+
}

0 commit comments

Comments
 (0)