klsince commented on code in PR #9681:
URL: https://github.com/apache/pinot/pull/9681#discussion_r1008716012
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskExecutor.java:
##########
@@ -177,12 +178,24 @@ private void pushSegment(String tableName, Map<String,
String> taskConfigs, URI
outputSegmentDirURI =
URI.create(taskConfigs.get(BatchConfigProperties.OUTPUT_SEGMENT_DIR_URI));
}
PinotFS outputFileFS =
SegmentGenerationAndPushTaskUtils.getOutputPinotFS(taskConfigs,
outputSegmentDirURI);
+ boolean closeOutputFSOnExit = true;
Review Comment:
It seems a bit error prone to me to use this flag to tell if the fs is from
a cache of a factory, e.g. the getOutputPinotFS () can return LOCAL_PINOT_FS
and it is closed (although it's a noop).
How about always return a new instance from getOutputPinotFS(), so it should
be closed all the time after use. It incurs some overhead but less error prone,
and it's for task generator/executor (batching processing) so the overhead can
be tolerable.
--
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]
