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/07/15 19:58:51 UTC
Review Request 50082: SAMZA-973: Disk Quotas: clamp max delay,
better measure processing time
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50082/
-----------------------------------------------------------
Review request for samza.
Repository: samza
Description
-------
The delay time for the disk quotas feature is currently unbounded which
can cause delays to become excessive e.g. in the case of a badly time GC
pause. This change provides the ability to set a maximum delay, which
defaults to 1 second.
This patch also improves the measurement of processing time. The process
method in RunLoop potentially blocks waiting to receive a new message.
This time should obviously not be included in the delay calculations.
Instead we split out message polling and process and apply delay
calculations only to the processing time.
Diffs
-----
samza-core/src/main/java/org/apache/samza/util/ThrottlingExecutor.java 214cefd4e8698fada6fc1bb14ab79be6afb27b9d
samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 18c09224bbae959342daf9b2b7a7d971cc224f48
samza-core/src/test/java/org/apache/samza/util/TestThrottlingExecutor.java 26590507b9c72a8c64171aeb1e5b7c3d5c24c41a
Diff: https://reviews.apache.org/r/50082/diff/
Testing
-------
Thanks,
Chris Pettitt
Re: Review Request 50082: SAMZA-973: Disk Quotas: clamp max delay,
better measure processing time
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50082/#review142457
-----------------------------------------------------------
Ship it!
+1. lgtm.
- Yi Pan (Data Infrastructure)
On July 15, 2016, 7:59 p.m., Chris Pettitt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50082/
> -----------------------------------------------------------
>
> (Updated July 15, 2016, 7:59 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> The delay time for the disk quotas feature is currently unbounded which
> can cause delays to become excessive e.g. in the case of a badly time GC
> pause. This change provides the ability to set a maximum delay, which
> defaults to 1 second.
>
> This patch also improves the measurement of processing time. The process
> method in RunLoop potentially blocks waiting to receive a new message.
> This time should obviously not be included in the delay calculations.
> Instead we split out message polling and process and apply delay
> calculations only to the processing time.
>
>
> Diffs
> -----
>
> samza-core/src/main/java/org/apache/samza/util/ThrottlingExecutor.java 214cefd4e8698fada6fc1bb14ab79be6afb27b9d
> samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 18c09224bbae959342daf9b2b7a7d971cc224f48
> samza-core/src/test/java/org/apache/samza/util/TestThrottlingExecutor.java 26590507b9c72a8c64171aeb1e5b7c3d5c24c41a
>
> Diff: https://reviews.apache.org/r/50082/diff/
>
>
> Testing
> -------
>
> gradle test
>
>
> Thanks,
>
> Chris Pettitt
>
>
Re: Review Request 50082: SAMZA-973: Disk Quotas: clamp max delay,
better measure processing time
Posted by Chris Pettitt <cp...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50082/
-----------------------------------------------------------
(Updated July 18, 2016, 3:29 p.m.)
Review request for samza.
Repository: samza
Description
-------
The delay time for the disk quotas feature is currently unbounded which
can cause delays to become excessive e.g. in the case of a badly time GC
pause. This change provides the ability to set a maximum delay, which
defaults to 1 second.
This patch also improves the measurement of processing time. The process
method in RunLoop potentially blocks waiting to receive a new message.
This time should obviously not be included in the delay calculations.
Instead we split out message polling and process and apply delay
calculations only to the processing time.
Diffs (updated)
-----
samza-core/src/main/java/org/apache/samza/util/ThrottlingExecutor.java 214cefd4e8698fada6fc1bb14ab79be6afb27b9d
samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 18c09224bbae959342daf9b2b7a7d971cc224f48
samza-core/src/test/java/org/apache/samza/util/TestThrottlingExecutor.java 26590507b9c72a8c64171aeb1e5b7c3d5c24c41a
Diff: https://reviews.apache.org/r/50082/diff/
Testing
-------
gradle test
Thanks,
Chris Pettitt
Re: Review Request 50082: SAMZA-973: Disk Quotas: clamp max delay,
better measure processing time
Posted by Chris Pettitt <cp...@linkedin.com>.
> On July 15, 2016, 10:52 p.m., Xinyu Liu wrote:
> > samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala, line 80
> > <https://reviews.apache.org/r/50082/diff/1/?file=1445036#file1445036line80>
> >
> > Shall we move this line in the process() function?
I'll move it into process. BTW, the current location preserves where it was relative to other code prior to this change. Moving it slightly changes the behavior, but appears to be closer to the original intention.
- Chris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50082/#review142459
-----------------------------------------------------------
On July 15, 2016, 7:59 p.m., Chris Pettitt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50082/
> -----------------------------------------------------------
>
> (Updated July 15, 2016, 7:59 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> The delay time for the disk quotas feature is currently unbounded which
> can cause delays to become excessive e.g. in the case of a badly time GC
> pause. This change provides the ability to set a maximum delay, which
> defaults to 1 second.
>
> This patch also improves the measurement of processing time. The process
> method in RunLoop potentially blocks waiting to receive a new message.
> This time should obviously not be included in the delay calculations.
> Instead we split out message polling and process and apply delay
> calculations only to the processing time.
>
>
> Diffs
> -----
>
> samza-core/src/main/java/org/apache/samza/util/ThrottlingExecutor.java 214cefd4e8698fada6fc1bb14ab79be6afb27b9d
> samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 18c09224bbae959342daf9b2b7a7d971cc224f48
> samza-core/src/test/java/org/apache/samza/util/TestThrottlingExecutor.java 26590507b9c72a8c64171aeb1e5b7c3d5c24c41a
>
> Diff: https://reviews.apache.org/r/50082/diff/
>
>
> Testing
> -------
>
> gradle test
>
>
> Thanks,
>
> Chris Pettitt
>
>
Re: Review Request 50082: SAMZA-973: Disk Quotas: clamp max delay,
better measure processing time
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50082/#review142459
-----------------------------------------------------------
Fix it, then Ship it!
samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala (line 79)
<https://reviews.apache.org/r/50082/#comment208023>
Shall we move this line in the process() function?
- Xinyu Liu
On July 15, 2016, 7:59 p.m., Chris Pettitt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50082/
> -----------------------------------------------------------
>
> (Updated July 15, 2016, 7:59 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> The delay time for the disk quotas feature is currently unbounded which
> can cause delays to become excessive e.g. in the case of a badly time GC
> pause. This change provides the ability to set a maximum delay, which
> defaults to 1 second.
>
> This patch also improves the measurement of processing time. The process
> method in RunLoop potentially blocks waiting to receive a new message.
> This time should obviously not be included in the delay calculations.
> Instead we split out message polling and process and apply delay
> calculations only to the processing time.
>
>
> Diffs
> -----
>
> samza-core/src/main/java/org/apache/samza/util/ThrottlingExecutor.java 214cefd4e8698fada6fc1bb14ab79be6afb27b9d
> samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 18c09224bbae959342daf9b2b7a7d971cc224f48
> samza-core/src/test/java/org/apache/samza/util/TestThrottlingExecutor.java 26590507b9c72a8c64171aeb1e5b7c3d5c24c41a
>
> Diff: https://reviews.apache.org/r/50082/diff/
>
>
> Testing
> -------
>
> gradle test
>
>
> Thanks,
>
> Chris Pettitt
>
>
Re: Review Request 50082: SAMZA-973: Disk Quotas: clamp max delay,
better measure processing time
Posted by Chris Pettitt <cp...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50082/
-----------------------------------------------------------
(Updated July 15, 2016, 7:59 p.m.)
Review request for samza.
Repository: samza
Description
-------
The delay time for the disk quotas feature is currently unbounded which
can cause delays to become excessive e.g. in the case of a badly time GC
pause. This change provides the ability to set a maximum delay, which
defaults to 1 second.
This patch also improves the measurement of processing time. The process
method in RunLoop potentially blocks waiting to receive a new message.
This time should obviously not be included in the delay calculations.
Instead we split out message polling and process and apply delay
calculations only to the processing time.
Diffs
-----
samza-core/src/main/java/org/apache/samza/util/ThrottlingExecutor.java 214cefd4e8698fada6fc1bb14ab79be6afb27b9d
samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 18c09224bbae959342daf9b2b7a7d971cc224f48
samza-core/src/test/java/org/apache/samza/util/TestThrottlingExecutor.java 26590507b9c72a8c64171aeb1e5b7c3d5c24c41a
Diff: https://reviews.apache.org/r/50082/diff/
Testing (updated)
-------
gradle test
Thanks,
Chris Pettitt