xiangfu0 commented on code in PR #20115:
URL: https://github.com/apache/pinot/pull/20115#discussion_r4448063567
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -1546,6 +1546,11 @@ public static class Server {
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";
+ // Disable automatic creation to require a pre-existing data directory at
server startup.
+ public static final String INSTANCE_DATA_DIR_CREATE_IF_MISSING =
"dataDir.createIfMissing";
Review Comment:
Addressed in a2c0f884d7. Renamed the flag to
`pinot.server.instance.dataDir.failOnMissing`, defaulting to `false`, and
updated the constants, getter, guard, error message, tests, and example config.
Missing directories are still created by default; setting the flag to `true`
makes startup fail. All 11 focused tests and the four affected-module precommit
checks pass.
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java:
##########
@@ -204,8 +205,10 @@ ServerIngestionOomProtectionManager.ServerThrottleState
getServerIngestionOomPro
}
@VisibleForTesting
- void initInstanceDataDir(File instanceDataDir) {
+ void initInstanceDataDir(File instanceDataDir, boolean createIfMissing) {
Review Comment:
Addressed in a2c0f884d7. Both initialization helpers now take no arguments
and read their paths and the flag from `_instanceDataManagerConfig`. The tests
initialize a real config with temporary paths and the desired flag.
`ensureDirectoryWritable(File, String)` remains the shared parameterized
utility.
##########
pinot-tools/src/main/resources/conf/pinot-server.conf:
##########
@@ -37,6 +37,9 @@ pinot.server.adminapi.port=8097
# Pinot Server Data Directory
pinot.server.instance.dataDir=/tmp/pinot/data/server/index
+# Set to false to fail startup if dataDir is missing, for deployments that
prepare disks separately.
+# This checks directory existence, not whether the intended disk is mounted.
Review Comment:
Clarified in a2c0f884d7: the startup guard applies only to the instance
`dataDir`. The example config and PR description now explicitly note that
`consumerDir` and tier data directories are still created on demand. The
mount-readiness limitation remains documented.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
