Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-20 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review143037 --- Ship it! Ship It! - Yi Pan (Data Infrastructure) On July

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-19 Thread Jagadish Venkatraman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/ --- (Updated July 19, 2016, 6:48 a.m.) Review request for samza, Boris Shkolnik,

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-19 Thread Jagadish Venkatraman
> On July 15, 2016, 9:34 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsGetter.java, > > line 34 > > > > > > Just curious, if the default

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-15 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review142434 ---

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-15 Thread Chris Pettitt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review142417 --- Ship it! I would add at least a test to verify you can get a

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-13 Thread Jagadish Venkatraman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/ --- (Updated July 14, 2016, 2:37 a.m.) Review request for samza, Boris Shkolnik,

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-13 Thread Jagadish Venkatraman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/ --- (Updated July 14, 2016, 2:28 a.m.) Review request for samza, Boris Shkolnik,

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-13 Thread Jagadish Venkatraman
> On July 12, 2016, 5:22 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java, > > line 67 > > > > > > How stable is this across versions of

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-13 Thread Jagadish Venkatraman
> On July 13, 2016, 12:48 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java, > > line 63 > > > > > > [Info Question] If only supported in Linux, do

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-13 Thread Jagadish Venkatraman
> On July 12, 2016, 5:22 p.m., Chris Pettitt wrote: > > Biggest concern with this patch is that it seems to bake in a dependency on > > Linux. Is Samza only supported on Linux? > > > > Other than that, some minor stuff to be fixed, but no major issues. Thanks for the feedback Chris! I thought

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-12 Thread Fred Ji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review141989 --- Thanks for the RB. I have a few comments for clarification.

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-12 Thread Chris Pettitt
> On July 11, 2016, 6:47 p.m., Chris Pettitt wrote: > > Very high level question: I assume you looked at `ps -o rss` and > > disqualified it for some reason. Could you elaborate as to why? `ps` itself > > is certainly more portable than procfs (though `-o rss` is not part of the > > POSIX

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-12 Thread Chris Pettitt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review141915 --- Biggest concern with this patch is that it seems to bake in a

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-11 Thread Jagadish Venkatraman
> On July 11, 2016, 6:47 p.m., Chris Pettitt wrote: > > Very high level question: I assume you looked at `ps -o rss` and > > disqualified it for some reason. Could you elaborate as to why? `ps` itself > > is certainly more portable than procfs (though `-o rss` is not part of the > > POSIX

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-11 Thread Xinyu Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review141760 --- Fix it, then Ship it!

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-11 Thread Jake Maes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review141747 --- Ship it!

Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-11 Thread Chris Pettitt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review141745 --- Very high level question: I assume you looked at `ps -o rss` and

Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer

2016-07-11 Thread Jagadish Venkatraman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/ --- Review request for samza, Boris Shkolnik, Chris Pettitt, Jake Maes, Yi Pan