samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>
Subject Re: Review Request 34746: Adding new CoordinatorStreamMessage "SetContainerHostMapping" and LocalityManager (SAMZA-618)
Date Mon, 01 Jun 2015 23:58:55 GMT


> On May 30, 2015, 8:58 a.m., Yi Pan (Data Infrastructure) wrote:
> > samza-core/src/main/java/org/apache/samza/container/LocalityManager.java, line 62
> > <https://reviews.apache.org/r/34746/diff/2/?file=974783#file974783line62>
> >
> >     This would be invoked twice by checkpointManager and localityManager. Generally,
I felt that since checkpointManager and localityManager are referring to the same set of coordinatorSystemConsumer
and coordinatorSystemProducer, shouldn't the start/stop/register be in the same life cycle,
instead of being invoked twice separately?
> 
> Navina Ramesh wrote:
>     Yeah. Naveen and I discussed about this. CheckpointManager, ChangelogManager and
LocalityManger can all use the same CoordinatorStreamAccessor instance and we can have accessors
like getCoordinatorStreamProducer(), getCoordinatorStreamConsumer() interfaces.
>     We have to refactor the entire set of coordinator stream related code. I started
making these changes. But they are pretty extensive. This change will be a part of [SAMZA-678](https://issues.apache.org/jira/browse/SAMZA-678)

Got it. Thanks for explaining.


> On May 30, 2015, 8:58 a.m., Yi Pan (Data Infrastructure) wrote:
> > samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala, line
74
> > <https://reviews.apache.org/r/34746/diff/2/?file=974787#file974787line74>
> >
> >     Maybe, it is easier to wrap the three managers that all requires to initialize
the coordinatorSystemProducer/coordinatorSystemConsumer in a single CoordinatorStreamManager?
And CoordinatorStreamManager.getCheckpointManager()/getChangelogManager()/getLocalityManager()
would return the specific management function handler?
> 
> Navina Ramesh wrote:
>     I guess this is also a pattern we can follow during the refactoring. 
>     As mentioned above, I will add this refactoring as a part of [SAMZA-678](https://issues.apache.org/jira/browse/SAMZA-678)

Thanks!


- Yi


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


On May 30, 2015, 11:49 p.m., Navina Ramesh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34746/
> -----------------------------------------------------------
> 
> (Updated May 30, 2015, 11:49 p.m.)
> 
> 
> Review request for samza, Chris Riccomini, Guozhang Wang, Yi Pan (Data Infrastructure),
and Naveen Somasundaram.
> 
> 
> Bugs: SAMZA-618
>     https://issues.apache.org/jira/browse/SAMZA-618
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Adding Locality Manager file
> 
> 
> reading in JC and writing from containers
> 
> 
> After SAMZA-686 changes
> 
> 
> Fixing stylechecks
> 
> 
> Correcting when coordinator system accessors start & stop
> 
> 
> Corrected documentation
> 
> 
> Diffs
> -----
> 
>   checkstyle/import-control.xml 5f8e103a2e43f96518b20de1c7cbd84e0af24842 
>   samza-core/src/main/java/org/apache/samza/container/LocalityManager.java PRE-CREATION

>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java
0988dedc3e8ad1b4080fb89dfff7c6f95fba8b67 
>   samza-core/src/main/java/org/apache/samza/job/model/JobModel.java fa113e12080384586b329c82133bc0601b855ae5

>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 50e53fbcb55c4e9176bf29217a341b195c96d762

>   samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala 5b43b58a851c363846b433ebd589ce6dd5c5c932

>   samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala a7fa0857d1243f5a24e4550a39ee230fbd7705bb

> 
> Diff: https://reviews.apache.org/r/34746/diff/
> 
> 
> Testing
> -------
> 
> Used a sample job to test it locally and also, by setting up YARN on 3 machines. 
> Verified that the message is correctly written and consumed from the Coordinator Stream
> 
> 
> Thanks,
> 
> Navina Ramesh
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message