orange475 commented on code in PR #1717:
URL: https://github.com/apache/samza/pull/1717#discussion_r2208816787
##########
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:
It will throw an exception and finally destroy the executable. This is to
avoid infinite wait. We could also remove this TTL to just call
executable.waitFor() or with a longer default time. @bringhurst what's your
thought on this?
--
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]
