You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Chris Pettitt <cp...@linkedin.com> on 2016/03/30 21:43:40 UTC
Review Request 45504: SAMZA-924: Add disk space monitoring
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45504/
-----------------------------------------------------------
Review request for samza.
Repository: samza
Description
-------
This change introduces the measurement of disk usage for selected
directories (currently those used by stores only). The feature is off by
default, but can be enabled by setting "container.disk.poll.interval.ms"
to a non-zero value.
Diffs
-----
samza-core/src/main/java/org/apache/samza/container/disk/DiskSpaceMonitor.java PRE-CREATION
samza-core/src/main/java/org/apache/samza/container/disk/PollingScanDiskSpaceMonitor.java PRE-CREATION
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala bcbc90a0a460f8733e6d3a50dbc33f3720cad7d0
samza-core/src/main/scala/org/apache/samza/container/SamzaContainerMetrics.scala 6fae6509d177cc3a54dac9ad1d3e5cc479f4a4f5
samza-core/src/test/java/org/apache/samza/container/disk/TestPollingScanDiskSpaceMonitor.java PRE-CREATION
Diff: https://reviews.apache.org/r/45504/diff/
Testing
-------
- Added tests for our disk space monitoring implementation.
- Verified metrics are correctly reported and updated when enabling the feature
- Verified metrics are not attached to the container when the feature is disabled
Perf testing for this feature is still pending and is a requirement for
this to be committed.
Thanks,
Chris Pettitt
Re: Review Request 45504: SAMZA-924: Add disk space monitoring
Posted by Chris Pettitt <cp...@linkedin.com>.
> On April 7, 2016, 9:14 p.m., Boris Shkolnik wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, line 398
> > <https://reviews.apache.org/r/45504/diff/1/?file=1319902#file1319902line398>
> >
> > Totally agree with your comment - it feels strange to hard code this stuff (although that what we do in TaskStorageManager).
> > May be it would be cleaner to add a getStoreLocation() method to the TaskStorageManager and call it for each taskInstance?
Thanks Boris. Actually that was not my comment. I moved this chunk out of a loop (a little further down in the diff) since we only need to evaluate it once.
- Chris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45504/#review127660
-----------------------------------------------------------
On March 30, 2016, 7:43 p.m., Chris Pettitt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45504/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 7:43 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> This change introduces the measurement of disk usage for selected
> directories (currently those used by stores only). The feature is off by
> default, but can be enabled by setting "container.disk.poll.interval.ms"
> to a non-zero value.
>
>
> Diffs
> -----
>
> samza-core/src/main/java/org/apache/samza/container/disk/DiskSpaceMonitor.java PRE-CREATION
> samza-core/src/main/java/org/apache/samza/container/disk/PollingScanDiskSpaceMonitor.java PRE-CREATION
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala bcbc90a0a460f8733e6d3a50dbc33f3720cad7d0
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainerMetrics.scala 6fae6509d177cc3a54dac9ad1d3e5cc479f4a4f5
> samza-core/src/test/java/org/apache/samza/container/disk/TestPollingScanDiskSpaceMonitor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45504/diff/
>
>
> Testing
> -------
>
> - Added tests for our disk space monitoring implementation.
> - Verified metrics are correctly reported and updated when enabling the feature
> - Verified metrics are not attached to the container when the feature is disabled
>
> Perf testing for this feature is still pending and is a requirement for
> this to be committed.
>
>
> Thanks,
>
> Chris Pettitt
>
>
Re: Review Request 45504: SAMZA-924: Add disk space monitoring
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45504/#review127660
-----------------------------------------------------------
Ship it!
Please see the comment. Up to you to decide to act on it or not.
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (line 398)
<https://reviews.apache.org/r/45504/#comment191047>
Totally agree with your comment - it feels strange to hard code this stuff (although that what we do in TaskStorageManager).
May be it would be cleaner to add a getStoreLocation() method to the TaskStorageManager and call it for each taskInstance?
- Boris Shkolnik
On March 30, 2016, 7:43 p.m., Chris Pettitt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45504/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 7:43 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> This change introduces the measurement of disk usage for selected
> directories (currently those used by stores only). The feature is off by
> default, but can be enabled by setting "container.disk.poll.interval.ms"
> to a non-zero value.
>
>
> Diffs
> -----
>
> samza-core/src/main/java/org/apache/samza/container/disk/DiskSpaceMonitor.java PRE-CREATION
> samza-core/src/main/java/org/apache/samza/container/disk/PollingScanDiskSpaceMonitor.java PRE-CREATION
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala bcbc90a0a460f8733e6d3a50dbc33f3720cad7d0
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainerMetrics.scala 6fae6509d177cc3a54dac9ad1d3e5cc479f4a4f5
> samza-core/src/test/java/org/apache/samza/container/disk/TestPollingScanDiskSpaceMonitor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45504/diff/
>
>
> Testing
> -------
>
> - Added tests for our disk space monitoring implementation.
> - Verified metrics are correctly reported and updated when enabling the feature
> - Verified metrics are not attached to the container when the feature is disabled
>
> Perf testing for this feature is still pending and is a requirement for
> this to be committed.
>
>
> Thanks,
>
> Chris Pettitt
>
>