You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Jakob Homan <jg...@apache.org> on 2014/02/13 18:58:30 UTC

Review Request 18088: SAMZA-146

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

Review request for samza.


Bugs: SAMZA-146
    https://issues.apache.org/jira/browse/SAMZA-146


Repository: samza


Description
-------

Improve performance on jobs with lots of topics.


Diffs
-----

  samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala cdba7fe 
  samza-core/src/main/scala/org/apache/samza/util/DoublingBackOff.scala PRE-CREATION 
  samza-core/src/test/scala/org/apache/samza/util/TestDoublingBackOff.scala PRE-CREATION 
  samza-test/src/main/java/org/apache/samza/system/mock/MockSystemConsumer.java 1e3457b 
  samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala ae3f663 
  samza-test/src/test/scala/org/apache/samza/test/performance/TestSamzaContainerPerformance.scala e5a676e 

Diff: https://reviews.apache.org/r/18088/diff/


Testing
-------

manual and unit.


Thanks,

Jakob Homan


Re: Review Request 18088: SAMZA-146

Posted by Chris Riccomini <cr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18088/#review34385
-----------------------------------------------------------

Ship it!


+1 Some white space nits, but other than that, looks great.


samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala
<https://reviews.apache.org/r/18088/#comment64470>

    Move to top of file with other val declarations (timeout, depletedQueueSizeThreshold, etc).
    
    Add a debug() line printing computed backoff.
    
    Also, seems kind of error prone to depend on this being lazy. Can we just compute in .start(), after fetchmap is guaranteed to be fully computed? Would require setting to var, which is annoying.



samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala
<https://reviews.apache.org/r/18088/#comment64469>

    delete extra \n.



samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala
<https://reviews.apache.org/r/18088/#comment64471>

    leave \n spacing here.


- Chris Riccomini


On Feb. 13, 2014, 5:58 p.m., Jakob Homan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18088/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2014, 5:58 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-146
>     https://issues.apache.org/jira/browse/SAMZA-146
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Improve performance on jobs with lots of topics.
> 
> 
> Diffs
> -----
> 
>   samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala cdba7fe 
>   samza-core/src/main/scala/org/apache/samza/util/DoublingBackOff.scala PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/util/TestDoublingBackOff.scala PRE-CREATION 
>   samza-test/src/main/java/org/apache/samza/system/mock/MockSystemConsumer.java 1e3457b 
>   samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala ae3f663 
>   samza-test/src/test/scala/org/apache/samza/test/performance/TestSamzaContainerPerformance.scala e5a676e 
> 
> Diff: https://reviews.apache.org/r/18088/diff/
> 
> 
> Testing
> -------
> 
> manual and unit.
> 
> 
> Thanks,
> 
> Jakob Homan
> 
>