orange475 commented on code in PR #1717:
URL: https://github.com/apache/samza/pull/1717#discussion_r2211811856
##########
samza-core/src/main/java/org/apache/samza/container/host/PosixCommandBasedStatisticsGetter.java:
##########
@@ -44,20 +47,71 @@ public class PosixCommandBasedStatisticsGetter implements
SystemStatisticsGetter
private List<String> getAllCommandOutput(String[] cmdArray) throws
IOException {
log.debug("Executing commands {}", Arrays.toString(cmdArray));
Process executable = Runtime.getRuntime().exec(cmdArray);
- BufferedReader processReader;
List<String> psOutput = new ArrayList<>();
- processReader = new BufferedReader(new
InputStreamReader(executable.getInputStream()));
- String line;
- while ((line = processReader.readLine()) != null) {
- if (!line.isEmpty()) {
- psOutput.add(line);
+ try (BufferedReader processReader = new BufferedReader(new
InputStreamReader(executable.getInputStream()));
+ BufferedReader errorReader = new BufferedReader(new
InputStreamReader(executable.getErrorStream()))) {
+
+ // Read output stream
+ String line;
+ while ((line = processReader.readLine()) != null) {
+ if (!line.isEmpty()) {
+ psOutput.add(line);
+ }
+ }
+
+ // Consume error stream to prevent blocking
+ consumeErrorStream(errorReader, cmdArray);
+
+ // Wait for the process to complete to prevent resource leak
+ try {
+ boolean finished = executable.waitFor(COMMAND_TIMEOUT_SECONDS,
TimeUnit.SECONDS);
Review Comment:
I dont think this would be necessary at this point as we don't want to
expose this to users for tuning. Make it configurable would also require a
wider refactoring.
--
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: commits-unsubscr...@samza.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
