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/01 21:07:04 UTC
Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/
-----------------------------------------------------------
Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
Repository: samza
Description
-------
As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
Diffs
-----
samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
Diff: https://reviews.apache.org/r/50670/diff/
Testing
-------
Thanks,
Jagadish Venkatraman
Re: Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/#review144393
-----------------------------------------------------------
Ship it!
+1
- Xinyu Liu
On Aug. 1, 2016, 10:21 p.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50670/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2016, 10:21 p.m.)
>
>
> Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
>
>
> Repository: samza
>
>
> Description
> -------
>
> As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
>
> This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
>
>
> Diffs
> -----
>
> samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50670/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>
Re: Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/#review144562
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Pan (Data Infrastructure)
On Aug. 1, 2016, 11:29 p.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50670/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2016, 11:29 p.m.)
>
>
> Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
>
>
> Repository: samza
>
>
> Description
> -------
>
> As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
>
> This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
>
>
> Diffs
> -----
>
> samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50670/diff/
>
>
> Testing
> -------
>
> 1. Build succeeds and unit tests pass.
> 2. Verified with a sample test job that metrics were actually flowing using JvisualVM.
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>
Re: Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
Posted by Jagadish Venkatraman <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/
-----------------------------------------------------------
(Updated Aug. 1, 2016, 11:29 p.m.)
Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
Repository: samza
Description
-------
As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
Diffs
-----
samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
Diff: https://reviews.apache.org/r/50670/diff/
Testing (updated)
-------
1. Build succeeds and unit tests pass.
2. Verified with a sample test job that metrics were actually flowing using JvisualVM.
Thanks,
Jagadish Venkatraman
Re: Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
Posted by Jagadish Venkatraman <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/
-----------------------------------------------------------
(Updated Aug. 1, 2016, 10:21 p.m.)
Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
Changes
-------
address Xinyu's review feedback.
Repository: samza
Description
-------
As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
Diffs (updated)
-----
samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
Diff: https://reviews.apache.org/r/50670/diff/
Testing
-------
Thanks,
Jagadish Venkatraman
Re: Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/#review144387
-----------------------------------------------------------
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala (line 78)
<https://reviews.apache.org/r/50670/#comment210409>
Seems jvm is started here again (see it in some other refactored code)
btw, seems Jvm metrics is also started in SamzaContainer. Do we run mulitple jvm stats? It might have some performance implications. Could you please take a look? Thanks.
- Xinyu Liu
On Aug. 1, 2016, 9:07 p.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50670/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2016, 9:07 p.m.)
>
>
> Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
>
>
> Repository: samza
>
>
> Description
> -------
>
> As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
>
> This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
>
>
> Diffs
> -----
>
> samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50670/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>
Re: Review Request 50670: SAMZA-991: Continue to report
SamzaAppMasterMetrics
Posted by Jagadish Venkatraman <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50670/#review144391
-----------------------------------------------------------
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala (line 78)
<https://reviews.apache.org/r/50670/#comment210420>
Good point, fixed this one! (The SamzaContainer however, does not run on the same process as the SamzaAppMaster.)
- Jagadish Venkatraman
On Aug. 1, 2016, 9:07 p.m., Jagadish Venkatraman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50670/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2016, 9:07 p.m.)
>
>
> Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and Xinyu Liu.
>
>
> Repository: samza
>
>
> Description
> -------
>
> As a part of the AM-JC refactoring, we took out some metrics from SamzaAppMasterMetrics and moved them to samza-core. However, for existing jobs that rely on metrics for their monitoring/alerting, changing metric-names may break the jobs.
>
> This change ensures that we still report basic metrics like failed containers, running containers, job health from Samza yarn for backwards-compat reasons.
>
>
> Diffs
> -----
>
> samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java 1fac7f4e656be2d83f7448a1ae498ffe32f6f6db
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50670/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jagadish Venkatraman
>
>