> 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 > >
