You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Jagadish Venkatraman <ja...@gmail.com> on 2016/08/16 02:18:41 UTC
Review Request 51126: SAMZA 998: Documentation updates for refactored
Job Coordinator
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51126/
-----------------------------------------------------------
Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), and Navina Ramesh.
Repository: samza
Description
-------
This documents the new config properties that were changed as a result of SAMZA-680. Some config properties that were specific to Yarn have been moved to the "cluster manager" namespace.
Question:
1.Is there a better prefix name we can come up with for config properties in the new name-space - instead of "cluster-manager.*" , how about job.*?
Suggestions for better config property names are welcome :-)
Diffs
-----
docs/learn/documentation/versioned/jobs/configuration-table.html 54c52981c3055b398ee60af50eeaf2592ed0e64f
samza-core/src/main/java/org/apache/samza/config/ClusterManagerConfig.java dafd7a7ac3f7c38aeab68de2dabfe09dde082388
Diff: https://reviews.apache.org/r/51126/diff/
Testing
-------
1. Generated html builds clean.
2. Built and tested the Samza website locally - `? docs git:(doc-updates) ? bundle exec jekyll serve --watch --baseurl`
Thanks,
Jagadish Venkatraman
Re: Review Request 51126: SAMZA 998: Documentation updates for
refactored Job Coordinator
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51126/#review148601
-----------------------------------------------------------
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1357)
<https://reviews.apache.org/r/51126/#comment216055>
One thing I learned from the Offspring discussion is that we use "." denominator in a free-style, which Offspring really does not like. We should follow the scoping of object configuration when using ".", not just use it as a denominator between words.
There are tons of violations in our current config. But it would be good to avoid introducing new config w/ the mistake. Not necessarily to be included in this review and should be kept in mind.
- Yi Pan (Data Infrastructure)
On Aug. 16, 2016, 2:18 a.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51126/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2016, 2:18 a.m.)
>
>
> Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), and Navina Ramesh.
>
>
> Repository: samza
>
>
> Description
> -------
>
> This documents the new config properties that were changed as a result of SAMZA-680. Some config properties that were specific to Yarn have been moved to the "cluster manager" namespace.
>
> Question:
> 1.Is there a better prefix name we can come up with for config properties in the new name-space - instead of "cluster-manager.*" , how about job.*?
>
> Suggestions for better config property names are welcome :-)
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/configuration-table.html 54c52981c3055b398ee60af50eeaf2592ed0e64f
> samza-core/src/main/java/org/apache/samza/config/ClusterManagerConfig.java dafd7a7ac3f7c38aeab68de2dabfe09dde082388
>
> Diff: https://reviews.apache.org/r/51126/diff/
>
>
> Testing
> -------
>
> 1. Generated html builds clean.
> 2. Built and tested the Samza website locally - `? docs git:(doc-updates) ? bundle exec jekyll serve --watch --baseurl`
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>
Re: Review Request 51126: SAMZA 998: Documentation updates for
refactored Job Coordinator
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51126/#review149579
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Pan (Data Infrastructure)
On Sept. 16, 2016, 9:45 p.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51126/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2016, 9:45 p.m.)
>
>
> Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), and Navina Ramesh.
>
>
> Repository: samza
>
>
> Description
> -------
>
> This documents the new config properties that were changed as a result of SAMZA-680. Some config properties that were specific to Yarn have been moved to the "cluster manager" namespace.
>
> Question:
> 1.Is there a better prefix name we can come up with for config properties in the new name-space - instead of "cluster-manager.*" , how about job.*?
>
> Suggestions for better config property names are welcome :-)
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/configuration-table.html 54c52981c3055b398ee60af50eeaf2592ed0e64f
> samza-core/src/main/java/org/apache/samza/config/ClusterManagerConfig.java dafd7a7ac3f7c38aeab68de2dabfe09dde082388
>
> Diff: https://reviews.apache.org/r/51126/diff/
>
>
> Testing
> -------
>
> 1. Generated html builds clean.
> 2. Built and tested the Samza website locally - `? docs git:(doc-updates) ? bundle exec jekyll serve --watch --baseurl`
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>
Re: Review Request 51126: SAMZA 998: Documentation updates for
refactored Job Coordinator
Posted by Jagadish Venkatraman <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51126/
-----------------------------------------------------------
(Updated Sept. 16, 2016, 9:45 p.m.)
Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), and Navina Ramesh.
Changes
-------
1.Address Yi's review feedback.
2.Re-organize some configs
3.Make host affinity config: job.host-affinity.enabled.
Repository: samza
Description
-------
This documents the new config properties that were changed as a result of SAMZA-680. Some config properties that were specific to Yarn have been moved to the "cluster manager" namespace.
Question:
1.Is there a better prefix name we can come up with for config properties in the new name-space - instead of "cluster-manager.*" , how about job.*?
Suggestions for better config property names are welcome :-)
Diffs (updated)
-----
docs/learn/documentation/versioned/jobs/configuration-table.html 54c52981c3055b398ee60af50eeaf2592ed0e64f
samza-core/src/main/java/org/apache/samza/config/ClusterManagerConfig.java dafd7a7ac3f7c38aeab68de2dabfe09dde082388
Diff: https://reviews.apache.org/r/51126/diff/
Testing
-------
1. Generated html builds clean.
2. Built and tested the Samza website locally - `? docs git:(doc-updates) ? bundle exec jekyll serve --watch --baseurl`
Thanks,
Jagadish Venkatraman
Re: Review Request 51126: SAMZA 998: Documentation updates for
refactored Job Coordinator
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51126/#review148030
-----------------------------------------------------------
Fix it, then Ship it!
Overall lgtm. Just one comment on the "soon to be deprecated"
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1490)
<https://reviews.apache.org/r/51126/#comment215393>
Do we still mandate this configuration variable? If not, let's change it to:
This *is* deprecated in favor of ...
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1499)
<https://reviews.apache.org/r/51126/#comment215394>
Same here.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1508)
<https://reviews.apache.org/r/51126/#comment215395>
Same here.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1517)
<https://reviews.apache.org/r/51126/#comment215396>
Also here.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1569)
<https://reviews.apache.org/r/51126/#comment215397>
Here as well.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1578)
<https://reviews.apache.org/r/51126/#comment215398>
Same.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1587)
<https://reviews.apache.org/r/51126/#comment215399>
Same here.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 1597)
<https://reviews.apache.org/r/51126/#comment215400>
Same.
- Yi Pan (Data Infrastructure)
On Aug. 16, 2016, 2:18 a.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51126/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2016, 2:18 a.m.)
>
>
> Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), and Navina Ramesh.
>
>
> Repository: samza
>
>
> Description
> -------
>
> This documents the new config properties that were changed as a result of SAMZA-680. Some config properties that were specific to Yarn have been moved to the "cluster manager" namespace.
>
> Question:
> 1.Is there a better prefix name we can come up with for config properties in the new name-space - instead of "cluster-manager.*" , how about job.*?
>
> Suggestions for better config property names are welcome :-)
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/configuration-table.html 54c52981c3055b398ee60af50eeaf2592ed0e64f
> samza-core/src/main/java/org/apache/samza/config/ClusterManagerConfig.java dafd7a7ac3f7c38aeab68de2dabfe09dde082388
>
> Diff: https://reviews.apache.org/r/51126/diff/
>
>
> Testing
> -------
>
> 1. Generated html builds clean.
> 2. Built and tested the Samza website locally - `? docs git:(doc-updates) ? bundle exec jekyll serve --watch --baseurl`
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>