jadami10 commented on a change in pull request #7961:
URL: https://github.com/apache/pinot/pull/7961#discussion_r777840740
##########
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
##########
@@ -374,6 +423,15 @@ public void start()
LOGGER.info("Starting Pinot server");
long startTimeMs = System.currentTimeMillis();
+ if (_serverConf.getProperty(Server.CONFIG_OF_STARTUP_ENABLE_TEMP_CLEANUP,
+ Server.DEFAULT_STARTUP_ENABLE_TEMP_CLEANUP)) {
+ File dataDir = new
File(_serverConf.getProperty(CommonConstants.Server.CONFIG_OF_INSTANCE_DATA_DIR));
+ // We use 3 hours as the cutoff time as a general heuristic for when
Review comment:
> Not a big fan of introducing a config just for cleaning up temp
directory but I'm fine with having a fixed threshold just as you mentioned:
delete for more than N hours. We shouldn't see multiple servers running on the
same machine in produciton.
@jackjlli preferred it this way, and I'm inclined to agree. There's already
a large number of configs, and ideally we remove this config in the future and
make this default behavior on server startup.
##########
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
##########
@@ -363,6 +377,41 @@ private void startupServiceStatusCheck(long endTimeMs) {
ServiceStatus.getStatusDescription());
}
+ /**
+ * Recursively deletes all data directories starting with tmp- last modified
before the startTime.
+ * @param startTime start time of the application
+ * @param dataDir data directory to start from
+ */
+ @VisibleForTesting
+ public static void deleteTempFilesSinceCutoffTime(long startTime, @Nonnull
File dataDir) {
+ if (!dataDir.exists() || !dataDir.isDirectory()) {
+ LOGGER.warn("Data directory {} does not exist or is not a directory",
dataDir);
+ return;
+ }
+ IOFileFilter beforeStartTimeFilter = new AgeFileFilter(startTime, true);
+ IOFileFilter tmpPrefixFilter = new PrefixFileFilter("tmp-");
Review comment:
it could, but that felt like a better and easy addition for a future PR
that requires it
--
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]