You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Sriram Subramanian <sr...@gmail.com> on 2013/09/12 23:52:31 UTC

Re: Review Request 13725: SAMZA-2

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13725/#review26064
-----------------------------------------------------------


1. I am not sure if the statement below is true. The chooser's update method can be called multiple times for the same topic partition before choose is called.

"A MessageChooser will receive, at most, one outstanding envelope per system/stream/partition combination. For example, if update is called for partition 7 of kafka.mystream, then update will not be called with an envelope from partition 7 of kafka.mystream until the previous envelope has been returned via the choose method."

2. Can you provide a usecase where register and start would be useful and what sort of computations can be done in these methods. All our choosers don't seem to do anything for these methods. I am trying to think what are the use cases this is useful Vs understandability of the api.

3. Could you add the assumptions made in the RoundRobin Scheduler about the container implementation.

- Sriram Subramanian


On Aug. 30, 2013, 5:47 p.m., Chris Riccomini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13725/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 5:47 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> added start, stop, and register to message chooser.
> 
> 
> adding docs for message chooser. swiching round robin chooser back to a queue.
> 
> 
> missed license in message chooser factory
> 
> 
> add apache licensing
> 
> 
> samza container was using message chooser, not message chooser factory. fixed.
> 
> 
> add stream chooser test. update stream chooser to invert priority due to bug.
> 
> 
> add round robin test. fix compile error in round robin chooser.
> 
> 
> add priority chooser test. fix bug in priority chooser that was reversing ordering.
> 
> 
> adding stream chooser. adding message chooser factory.
> 
> 
> adding priority chooser. moving default chooser to round robin chooser. adding config for chooser
> 
> 
> Diffs
> -----
> 
>   docs/learn/documentation/0.7.0/container/streams.md e755789407b294e02b399e71ba684c1d6dc314c6 
>   samza-api/src/main/java/org/apache/samza/system/MessageChooser.java 306b2902303c72f3d7a3eb313f55d7e88d21e00d 
>   samza-api/src/main/java/org/apache/samza/system/PriorityChooser.java PRE-CREATION 
>   samza-api/src/test/java/org/apache/samza/system/TestPriorityChooser.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/config/StreamChooserConfig.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala 0c742d83c2f60d2448a79376677713a1ff0b11ec 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 2d2efdd14c7680c29aad5f2a98349e2fc57cf9fe 
>   samza-core/src/main/scala/org/apache/samza/system/DefaultChooser.scala 5a72e7a3bfba0f06a5a98c6ba26865800d7780b9 
>   samza-core/src/main/scala/org/apache/samza/system/RoundRobinChooser.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/system/StreamChooser.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala b18f0cc5a21088a58db1c26ff43bba06dd3165ac 
>   samza-core/src/test/scala/org/apache/samza/system/TestRoundRobinChooser.scala PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/system/TestStreamChooser.scala PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/system/TestSystemConsumers.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13725/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Riccomini
> 
>


Re: Review Request 13725: SAMZA-2

Posted by Chris Riccomini <cr...@apache.org>.

> On Sept. 12, 2013, 9:52 p.m., Sriram Subramanian wrote:
> > 1. I am not sure if the statement below is true. The chooser's update method can be called multiple times for the same topic partition before choose is called.
> > 
> > "A MessageChooser will receive, at most, one outstanding envelope per system/stream/partition combination. For example, if update is called for partition 7 of kafka.mystream, then update will not be called with an envelope from partition 7 of kafka.mystream until the previous envelope has been returned via the choose method."
> > 
> > 2. Can you provide a usecase where register and start would be useful and what sort of computations can be done in these methods. All our choosers don't seem to do anything for these methods. I am trying to think what are the use cases this is useful Vs understandability of the api.
> > 
> > 3. Could you add the assumptions made in the RoundRobin Scheduler about the container implementation.

1. Hmm. Can you point out where/how the update method can be called more than once for a single SSP? This is a bug.

2. No I can't. I'm backing off this interface change. Will revert.

3. Yes, will do.


- Chris


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13725/#review26064
-----------------------------------------------------------


On Aug. 30, 2013, 5:47 p.m., Chris Riccomini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13725/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2013, 5:47 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> added start, stop, and register to message chooser.
> 
> 
> adding docs for message chooser. swiching round robin chooser back to a queue.
> 
> 
> missed license in message chooser factory
> 
> 
> add apache licensing
> 
> 
> samza container was using message chooser, not message chooser factory. fixed.
> 
> 
> add stream chooser test. update stream chooser to invert priority due to bug.
> 
> 
> add round robin test. fix compile error in round robin chooser.
> 
> 
> add priority chooser test. fix bug in priority chooser that was reversing ordering.
> 
> 
> adding stream chooser. adding message chooser factory.
> 
> 
> adding priority chooser. moving default chooser to round robin chooser. adding config for chooser
> 
> 
> Diffs
> -----
> 
>   docs/learn/documentation/0.7.0/container/streams.md e755789407b294e02b399e71ba684c1d6dc314c6 
>   samza-api/src/main/java/org/apache/samza/system/MessageChooser.java 306b2902303c72f3d7a3eb313f55d7e88d21e00d 
>   samza-api/src/main/java/org/apache/samza/system/PriorityChooser.java PRE-CREATION 
>   samza-api/src/test/java/org/apache/samza/system/TestPriorityChooser.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/config/StreamChooserConfig.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala 0c742d83c2f60d2448a79376677713a1ff0b11ec 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 2d2efdd14c7680c29aad5f2a98349e2fc57cf9fe 
>   samza-core/src/main/scala/org/apache/samza/system/DefaultChooser.scala 5a72e7a3bfba0f06a5a98c6ba26865800d7780b9 
>   samza-core/src/main/scala/org/apache/samza/system/RoundRobinChooser.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/system/StreamChooser.scala PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala b18f0cc5a21088a58db1c26ff43bba06dd3165ac 
>   samza-core/src/test/scala/org/apache/samza/system/TestRoundRobinChooser.scala PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/system/TestStreamChooser.scala PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/system/TestSystemConsumers.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13725/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Riccomini
> 
>