You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Navina Ramesh <nr...@linkedin.com> on 2015/05/28 04:26:28 UTC
Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/
-----------------------------------------------------------
Review request for samza, Chris Riccomini, Guozhang Wang, Yi Pan (Data Infrastructure), and Naveen Somasundaram.
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
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/#review86125
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Pan (Data Infrastructure)
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
>
>
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
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 (updated)
-----
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
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by Navina Ramesh <nr...@linkedin.com>.
> 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 27
> > <https://reviews.apache.org/r/34746/diff/2/?file=974783#file974783line27>
> >
> > nit: why is here a "*" import?
not intentional :)
> 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?
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)
> 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 79
> > <https://reviews.apache.org/r/34746/diff/2/?file=974783#file974783line79>
> >
> > What if the existingMapping == hostHttpAddress? I think that would be the "not moved" case?
Ah.. you are right. I will add the equality check.
> 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?
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)
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/#review85855
-----------------------------------------------------------
On May 29, 2015, 11:26 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34746/
> -----------------------------------------------------------
>
> (Updated May 29, 2015, 11:26 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
>
>
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
> 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
>
>
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/#review85855
-----------------------------------------------------------
Overall looks good to me. There is one comment related to the multiple invocation of coordinator consumer/producer lifecycle method. Sorry for comment and review in a late time.
samza-core/src/main/java/org/apache/samza/container/LocalityManager.java
<https://reviews.apache.org/r/34746/#comment137691>
nit: why is here a "*" import?
samza-core/src/main/java/org/apache/samza/container/LocalityManager.java
<https://reviews.apache.org/r/34746/#comment137693>
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?
samza-core/src/main/java/org/apache/samza/container/LocalityManager.java
<https://reviews.apache.org/r/34746/#comment137692>
What if the existingMapping == hostHttpAddress? I think that would be the "not moved" case?
samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala
<https://reviews.apache.org/r/34746/#comment137694>
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?
- Yi Pan (Data Infrastructure)
On May 29, 2015, 11:26 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34746/
> -----------------------------------------------------------
>
> (Updated May 29, 2015, 11:26 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
>
>
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/
-----------------------------------------------------------
(Updated May 29, 2015, 11:26 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 (updated)
-----
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
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by Navina Ramesh <nr...@linkedin.com>.
> On May 29, 2015, 8:38 p.m., Yan Fang wrote:
> > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java, line 503
> > <https://reviews.apache.org/r/34746/diff/1/?file=973284#file973284line503>
> >
> > Changing to "host" or "ip addree" is better. Because
> > 1) it's not "hostname".
> > 2) we use the "host" for the same value:
> >
> > protected void setHost(String host) {
> > messageMap.put("host", host);
> > }
Makes sense. I will change it to "ip".
> On May 29, 2015, 8:38 p.m., Yan Fang wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, line 629
> > <https://reviews.apache.org/r/34746/diff/1/?file=973286#file973286line629>
> >
> > catch the UnknownHostException ?
Mmm.. makes sense. It could be thrown. I will change it to handle the exception with a no-op.
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/#review85798
-----------------------------------------------------------
On May 29, 2015, 6:32 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34746/
> -----------------------------------------------------------
>
> (Updated May 29, 2015, 6:32 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
>
>
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by Yan Fang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/#review85798
-----------------------------------------------------------
samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java
<https://reviews.apache.org/r/34746/#comment137599>
Changing to "host" or "ip addree" is better. Because
1) it's not "hostname".
2) we use the "host" for the same value:
protected void setHost(String host) {
messageMap.put("host", host);
}
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala
<https://reviews.apache.org/r/34746/#comment137593>
catch the UnknownHostException ?
- Yan Fang
On May 29, 2015, 6:32 p.m., Navina Ramesh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34746/
> -----------------------------------------------------------
>
> (Updated May 29, 2015, 6:32 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
>
>
Re: Review Request 34746: Adding new CoordinatorStreamMessage
"SetContainerHostMapping" and LocalityManager (SAMZA-618)
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34746/
-----------------------------------------------------------
(Updated May 29, 2015, 6:32 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