Jackie-Jiang commented on code in PR #20115:
URL: https://github.com/apache/pinot/pull/20115#discussion_r4447182149
##########
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:
I'd prefer `pinot.server.instance.dataDir.failOnMissing`, defaulting to
`false`. That directly expresses the opt-in startup failure: `false` keeps the
existing automatic directory creation, and `true` fails when the directory is
missing. Please update the constants, getter, condition, error message, tests,
and example config consistently.
##########
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:
Can we make both `initInstanceDataDir()` and `initInstanceSegmentTarDir()`
take no arguments? The directory paths and the new flag are already available
through the `_instanceDataManagerConfig` member, so each helper can read its
inputs directly. Keep `ensureDirectoryWritable(File, String)` parameterized as
the shared utility, and update the tests to initialize the manager's config
with temporary paths and the desired flag.
--
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]
