You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by GitBox <gi...@apache.org> on 2021/09/23 21:20:14 UTC

[GitHub] [samza] ZitingShen opened a new pull request #1533: [WIP] Test if Travis build is broken

ZitingShen opened a new pull request #1533:
URL: https://github.com/apache/samza/pull/1533


   Test if Travis build is broken by https://github.com/apache/samza/pull/1530


-- 
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-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu merged pull request #1533: Fix container physical memory monitoring to fetch all children process RSS memory with 1 process

Posted by GitBox <gi...@apache.org>.
rmatharu merged pull request #1533:
URL: https://github.com/apache/samza/pull/1533


   


-- 
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-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu commented on a change in pull request #1533: Fix container physical memory monitoring to fetch all children process RSS memory with 1 process

Posted by GitBox <gi...@apache.org>.
rmatharu commented on a change in pull request #1533:
URL: https://github.com/apache/samza/pull/1533#discussion_r717111009



##########
File path: samza-core/src/main/java/org/apache/samza/container/host/PosixCommandBasedStatisticsGetter.java
##########
@@ -67,17 +43,17 @@ private String getCommandOutput(String[] cmdArray) throws IOException {
    */
   private List<String> getAllCommandOutput(String[] cmdArray) throws IOException {
     Process executable = Runtime.getRuntime().exec(cmdArray);
-    BufferedReader processReader = null;
-    List<String> psOutput;
+    BufferedReader processReader;
+    List<String> psOutput = new ArrayList<>();
 

Review comment:
       Mayb add a log.info("executing commands {0}, cmdArray);




-- 
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-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu merged pull request #1533: Fix container physical memory monitoring to fetch all children process RSS memory with 1 process

Posted by GitBox <gi...@apache.org>.
rmatharu merged pull request #1533:
URL: https://github.com/apache/samza/pull/1533


   


-- 
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-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu commented on a change in pull request #1533: Fix container physical memory monitoring to fetch all children process RSS memory with 1 process

Posted by GitBox <gi...@apache.org>.
rmatharu commented on a change in pull request #1533:
URL: https://github.com/apache/samza/pull/1533#discussion_r717111009



##########
File path: samza-core/src/main/java/org/apache/samza/container/host/PosixCommandBasedStatisticsGetter.java
##########
@@ -67,17 +43,17 @@ private String getCommandOutput(String[] cmdArray) throws IOException {
    */
   private List<String> getAllCommandOutput(String[] cmdArray) throws IOException {
     Process executable = Runtime.getRuntime().exec(cmdArray);
-    BufferedReader processReader = null;
-    List<String> psOutput;
+    BufferedReader processReader;
+    List<String> psOutput = new ArrayList<>();
 

Review comment:
       Mayb add a log.info("executing commands {0}, cmdArray);

##########
File path: samza-core/src/main/java/org/apache/samza/container/host/PosixCommandBasedStatisticsGetter.java
##########
@@ -86,28 +62,17 @@ private long getTotalPhysicalMemory() throws IOException {
     List<String> processIds = getAllCommandOutput(new String[]{"sh", "-c", "pgrep -P $PPID"});

Review comment:
       nit: rename method to getTotalPhysicalMemoryUsageBytes
   so that the purpose and the unit of the response is clear to the caller.




-- 
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-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu commented on a change in pull request #1533: Fix container physical memory monitoring to fetch all children process RSS memory with 1 process

Posted by GitBox <gi...@apache.org>.
rmatharu commented on a change in pull request #1533:
URL: https://github.com/apache/samza/pull/1533#discussion_r717111449



##########
File path: samza-core/src/main/java/org/apache/samza/container/host/PosixCommandBasedStatisticsGetter.java
##########
@@ -86,28 +62,17 @@ private long getTotalPhysicalMemory() throws IOException {
     List<String> processIds = getAllCommandOutput(new String[]{"sh", "-c", "pgrep -P $PPID"});

Review comment:
       nit: rename method to getTotalPhysicalMemoryUsageBytes
   so that the purpose and the unit of the response is clear to the caller.




-- 
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-unsubscribe@samza.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org