This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 3ac4b6e6c7b Add option to require an existing server data directory
(#20115)
3ac4b6e6c7b is described below
commit 3ac4b6e6c7b83e07d09a646e0fedc218eb03fb53
Author: Xiang Fu <[email protected]>
AuthorDate: Mon Nov 2 14:52:51 2026 -0800
Add option to require an existing server data directory (#20115)
* Add option to require an existing server data directory
* Address server data directory startup review feedback
---
.../starter/helix/HelixInstanceDataManager.java | 17 +++---
.../helix/HelixInstanceDataManagerConfig.java | 4 ++
.../helix/HelixInstanceDataManagerConfigTest.java | 17 ++++++
.../helix/HelixInstanceDataManagerTest.java | 68 +++++++++++++++++-----
.../apache/pinot/spi/utils/CommonConstants.java | 5 ++
.../src/main/resources/conf/pinot-server.conf | 4 ++
6 files changed, 93 insertions(+), 22 deletions(-)
diff --git
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
index 3b3e4fbfe29..df673ce8655 100644
---
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
+++
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
@@ -74,6 +74,7 @@ import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.plugin.PluginManager;
+import org.apache.pinot.spi.utils.CommonConstants.Server;
import org.apache.pinot.spi.utils.ConsumingSegmentConsistencyModeListener;
import org.apache.pinot.spi.utils.TimestampIndexUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
@@ -165,11 +166,8 @@ public class HelixInstanceDataManager implements
InstanceDataManager {
_segmentUploader = new
PinotFSSegmentUploader(_instanceDataManagerConfig.getSegmentStoreUri(),
ServerSegmentCompletionProtocolHandler.getSegmentUploadRequestTimeoutMs(),
serverMetrics);
- File instanceDataDir = new
File(_instanceDataManagerConfig.getInstanceDataDir());
- initInstanceDataDir(instanceDataDir);
-
- File instanceSegmentTarDir = new
File(_instanceDataManagerConfig.getInstanceSegmentTarDir());
- initInstanceSegmentTarDir(instanceSegmentTarDir);
+ initInstanceDataDir();
+ initInstanceSegmentTarDir();
// Initialize segment build time lease extender executor
SegmentBuildTimeLeaseExtender.initExecutor();
@@ -204,8 +202,12 @@ public class HelixInstanceDataManager implements
InstanceDataManager {
}
@VisibleForTesting
- void initInstanceDataDir(File instanceDataDir) {
+ void initInstanceDataDir() {
+ File instanceDataDir = new
File(_instanceDataManagerConfig.getInstanceDataDir());
if (!instanceDataDir.exists()) {
+
Preconditions.checkState(!_instanceDataManagerConfig.shouldFailOnMissingInstanceDataDir(),
+ "Instance data dir does not exist: %s (%s=true)", instanceDataDir,
+ Server.CONFIG_OF_INSTANCE_DATA_DIR_FAIL_ON_MISSING);
Preconditions.checkState(instanceDataDir.mkdirs(), "Failed to create
instance data dir: %s", instanceDataDir);
} else {
// Clean up the _tmp (consuming segments) dir and empty table data dir
@@ -232,7 +234,8 @@ public class HelixInstanceDataManager implements
InstanceDataManager {
}
@VisibleForTesting
- void initInstanceSegmentTarDir(File instanceSegmentTarDir) {
+ void initInstanceSegmentTarDir() {
+ File instanceSegmentTarDir = new
File(_instanceDataManagerConfig.getInstanceSegmentTarDir());
if (!instanceSegmentTarDir.exists()) {
Preconditions.checkState(instanceSegmentTarDir.mkdirs(), "Failed to
create instance segment tar dir: %s",
instanceSegmentTarDir);
diff --git
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
index d31a99d7945..c240ea22dec 100644
---
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
+++
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
@@ -199,6 +199,10 @@ public class HelixInstanceDataManagerConfig implements
InstanceDataManagerConfig
return _serverConfig.getProperty(INSTANCE_DATA_DIR,
DEFAULT_INSTANCE_DATA_DIR);
}
+ public boolean shouldFailOnMissingInstanceDataDir() {
+ return _serverConfig.getProperty(INSTANCE_DATA_DIR_FAIL_ON_MISSING,
DEFAULT_INSTANCE_DATA_DIR_FAIL_ON_MISSING);
+ }
+
@Override
public String getConsumerDir() {
return _serverConfig.getProperty(CONSUMER_DIR);
diff --git
a/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfigTest.java
b/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfigTest.java
index 9c70581fa24..46180044913 100644
---
a/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfigTest.java
+++
b/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfigTest.java
@@ -24,12 +24,29 @@ import
org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.testng.annotations.Test;
+import static
org.apache.pinot.spi.utils.CommonConstants.Server.CONFIG_OF_INSTANCE_DATA_DIR_FAIL_ON_MISSING;
+import static
org.apache.pinot.spi.utils.CommonConstants.Server.CONFIG_OF_INSTANCE_ID;
+import static
org.apache.pinot.spi.utils.CommonConstants.Server.INSTANCE_DATA_MANAGER_CONFIG_PREFIX;
import static org.apache.pinot.spi.utils.CommonConstants.Server.INSTANCE_ID;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
public class HelixInstanceDataManagerConfigTest {
+ @Test
+ public void testInstanceDataDirFailOnMissingConfig()
+ throws ConfigurationException {
+ PinotConfiguration serverConfig = new
PinotConfiguration(Map.of(CONFIG_OF_INSTANCE_ID, "testInstance"));
+ assertFalse(new
HelixInstanceDataManagerConfig(serverConfig.subset(INSTANCE_DATA_MANAGER_CONFIG_PREFIX))
+ .shouldFailOnMissingInstanceDataDir());
+
+ serverConfig.setProperty(CONFIG_OF_INSTANCE_DATA_DIR_FAIL_ON_MISSING,
"true");
+ assertTrue(new
HelixInstanceDataManagerConfig(serverConfig.subset(INSTANCE_DATA_MANAGER_CONFIG_PREFIX))
+ .shouldFailOnMissingInstanceDataDir());
+ }
+
@Test
public void testTierConfigsWithMultipleTiers() throws ConfigurationException
{
Map<String, Object> props = new HashMap<>();
diff --git
a/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerTest.java
b/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerTest.java
index b50aca813fb..1cedf716222 100644
---
a/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerTest.java
+++
b/pinot-server/src/test/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerTest.java
@@ -20,38 +20,55 @@ package org.apache.pinot.server.starter.helix;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Files;
+import java.util.Map;
import org.apache.commons.io.FileUtils;
-import org.mockito.Mockito;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.pinot.spi.env.PinotConfiguration;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import static
org.apache.pinot.spi.utils.CommonConstants.Server.INSTANCE_DATA_DIR;
+import static
org.apache.pinot.spi.utils.CommonConstants.Server.INSTANCE_DATA_DIR_FAIL_ON_MISSING;
+import static org.apache.pinot.spi.utils.CommonConstants.Server.INSTANCE_ID;
+import static
org.apache.pinot.spi.utils.CommonConstants.Server.INSTANCE_SEGMENT_TAR_DIR;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
import static org.testng.Assert.fail;
public class HelixInstanceDataManagerTest {
private HelixInstanceDataManager _helixInstanceDataManager;
+ private File _testDir;
private File _testInstanceDataDir;
private File _testInstanceSegmentTarDir;
@BeforeClass
- public void setUp() {
- _helixInstanceDataManager = Mockito.mock(HelixInstanceDataManager.class);
- _testInstanceDataDir = new File("testInstanceDataDir");
- _testInstanceSegmentTarDir = new File("testInstanceSegmentTarDir");
-
Mockito.doCallRealMethod().when(_helixInstanceDataManager).initInstanceDataDir(_testInstanceDataDir);
-
Mockito.doCallRealMethod().when(_helixInstanceDataManager).initInstanceSegmentTarDir(_testInstanceSegmentTarDir);
+ public void setUp()
+ throws Exception {
+ _testDir =
Files.createTempDirectory("pinot-instance-data-manager-test").toFile();
+ _testInstanceDataDir = new File(_testDir, "data");
+ _testInstanceSegmentTarDir = new File(_testDir, "segmentTar");
+ _helixInstanceDataManager = createInstanceDataManager(new
PinotConfiguration(Map.of(INSTANCE_ID, "testInstance",
+ INSTANCE_DATA_DIR, _testInstanceDataDir.toString(),
INSTANCE_SEGMENT_TAR_DIR,
+ _testInstanceSegmentTarDir.toString())));
}
- @AfterClass
+ @AfterClass(alwaysRun = true)
public void tearDown() {
- FileUtils.deleteQuietly(_testInstanceDataDir);
- FileUtils.deleteQuietly(_testInstanceSegmentTarDir);
+ FileUtils.deleteQuietly(_testDir);
+ }
+
+ private static HelixInstanceDataManager
createInstanceDataManager(PinotConfiguration config)
+ throws Exception {
+ HelixInstanceDataManager manager = new HelixInstanceDataManager();
+ FieldUtils.writeField(manager, "_instanceDataManagerConfig", new
HelixInstanceDataManagerConfig(config), true);
+ return manager;
}
@Test
@@ -60,17 +77,38 @@ public class HelixInstanceDataManagerTest {
FileUtils.deleteQuietly(_testInstanceDataDir);
}
- _helixInstanceDataManager.initInstanceDataDir(_testInstanceDataDir);
+ _helixInstanceDataManager.initInstanceDataDir();
assertTrue(_testInstanceDataDir.exists(), "Instance data directory should
be created.");
}
+ @Test
+ public void testInitInstanceDataDirFailsOnMissingDirectory()
+ throws Exception {
+ File tempDir = Files.createTempDirectory("pinot-data-dir-test").toFile();
+ File dataDir = new File(tempDir, "disk/data");
+ try {
+ HelixInstanceDataManager manager = createInstanceDataManager(new
PinotConfiguration(Map.of(INSTANCE_ID,
+ "testInstance", INSTANCE_DATA_DIR, dataDir.toString(),
INSTANCE_DATA_DIR_FAIL_ON_MISSING, true)));
+ IllegalStateException exception =
expectThrows(IllegalStateException.class, manager::initInstanceDataDir);
+ assertTrue(exception.getMessage().contains(dataDir.toString()));
+
assertTrue(exception.getMessage().contains("pinot.server.instance.dataDir.failOnMissing=true"));
+ assertFalse(dataDir.getParentFile().exists(), "Missing data directory
must not be created.");
+
+ assertTrue(dataDir.mkdirs(), "Failed to prepare data directory.");
+ manager.initInstanceDataDir();
+ assertTrue(dataDir.isDirectory());
+ } finally {
+ FileUtils.deleteDirectory(tempDir);
+ }
+ }
+
@Test
public void testInitInstanceDataDirCleansUpTempDirs() {
File tableDir = new File(_testInstanceDataDir, "tableA_OFFLINE");
assertTrue(tableDir.mkdirs(), "Failed to create temp directory.");
- _helixInstanceDataManager.initInstanceDataDir(_testInstanceDataDir);
+ _helixInstanceDataManager.initInstanceDataDir();
assertFalse(tableDir.exists(), "Temporary directory should be deleted.");
}
@@ -83,7 +121,7 @@ public class HelixInstanceDataManagerTest {
_testInstanceDataDir.setWritable(false);
try {
- _helixInstanceDataManager.initInstanceDataDir(_testInstanceDataDir);
+ _helixInstanceDataManager.initInstanceDataDir();
fail("Expected IllegalStateException due to non-writable directory.");
} catch (IllegalStateException e) {
// Expected exception
@@ -114,7 +152,7 @@ public class HelixInstanceDataManagerTest {
FileUtils.deleteQuietly(_testInstanceSegmentTarDir);
}
-
_helixInstanceDataManager.initInstanceSegmentTarDir(_testInstanceSegmentTarDir);
+ _helixInstanceDataManager.initInstanceSegmentTarDir();
assertTrue(_testInstanceSegmentTarDir.exists(), "Instance segment tar
directory should be created.");
}
@@ -127,7 +165,7 @@ public class HelixInstanceDataManagerTest {
_testInstanceSegmentTarDir.setWritable(false);
try {
-
_helixInstanceDataManager.initInstanceSegmentTarDir(_testInstanceSegmentTarDir);
+ _helixInstanceDataManager.initInstanceSegmentTarDir();
fail("Expected IllegalStateException due to non-writable directory.");
} catch (IllegalStateException e) {
// Expected exception
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index 0b494ba02b5..c979a58ee6e 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -1546,6 +1546,11 @@ public class CommonConstants {
public static final String DEFAULT_INSTANCE_BASE_DIR =
FileUtils.getTempDirectoryPath() + File.separator + "PinotServer";
public static final String DEFAULT_INSTANCE_DATA_DIR =
DEFAULT_INSTANCE_BASE_DIR + File.separator + "index";
+ // Require a pre-existing data directory at server startup when enabled.
+ public static final String INSTANCE_DATA_DIR_FAIL_ON_MISSING =
"dataDir.failOnMissing";
+ public static final String CONFIG_OF_INSTANCE_DATA_DIR_FAIL_ON_MISSING =
+ INSTANCE_DATA_MANAGER_CONFIG_PREFIX + "." +
INSTANCE_DATA_DIR_FAIL_ON_MISSING;
+ public static final boolean DEFAULT_INSTANCE_DATA_DIR_FAIL_ON_MISSING =
false;
public static final String CONSUMER_DIR = "consumerDir";
public static final String CONFIG_OF_CONSUMER_DIR =
INSTANCE_DATA_MANAGER_CONFIG_PREFIX + "." + CONSUMER_DIR;
public static final String INSTANCE_SEGMENT_TAR_DIR = "segmentTarDir";
diff --git a/pinot-tools/src/main/resources/conf/pinot-server.conf
b/pinot-tools/src/main/resources/conf/pinot-server.conf
index 066f46651c7..26eae833b8d 100644
--- a/pinot-tools/src/main/resources/conf/pinot-server.conf
+++ b/pinot-tools/src/main/resources/conf/pinot-server.conf
@@ -37,6 +37,10 @@ pinot.server.adminapi.port=8097
# Pinot Server Data Directory
pinot.server.instance.dataDir=/tmp/pinot/data/server/index
+# Set to true to fail startup if dataDir is missing, for deployments that
prepare disks separately.
+# Applies only to the instance dataDir; consumerDir and tier dataDirs are
still created on demand.
+# This checks directory existence, not whether the intended disk is mounted.
+# pinot.server.instance.dataDir.failOnMissing=false
# Pinot Server Temporary Segment Tar Directory
pinot.server.instance.segmentTarDir=/tmp/pinot/data/server/segmentTar
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
