You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Zhijie Shen <zs...@hortonworks.com> on 2014/04/21 00:48:09 UTC
Review Request 20514: SAMZA-218 Show container up-time in YARN AM
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/
-----------------------------------------------------------
Review request for samza.
Repository: samza
Description
-------
SAMZA-218 Show container up-time in YARN AM
Diffs
-----
samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
Diff: https://reviews.apache.org/r/20514/diff/
Testing
-------
Thanks,
Zhijie Shen
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Chris Riccomini <cr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/#review41359
-----------------------------------------------------------
I'm OK with ISO, but I agree it's a little hard to read. I'll leave it up to you. I think we need to add joda to the build.gradle stuff now, since we're depending on it explicitly as a compile-time dependency.
Anyone else have any preferences on time? It seems like we can use a DateFormatter to make any custom time format we want.
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala
<https://reviews.apache.org/r/20514/#comment74778>
Need to add joda time as an explicit compile-time dependency in build.gradle now, since we're using it.
- Chris Riccomini
On April 24, 2014, 7:11 a.m., Zhijie Shen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20514/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 7:11 a.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-218 Show container up-time in YARN AM
>
>
> Diffs
> -----
>
> samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
> samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
>
> Diff: https://reviews.apache.org/r/20514/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhijie Shen
>
>
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN
AM
Posted by Chris Riccomini <cr...@linkedin.com>.
Oops, sorry. Want to commit it? :)
On 4/30/14 10:43 PM, "Zhijie Shen" <zs...@hortonworks.com> wrote:
>
>
>> On April 28, 2014, 4:07 p.m., Chris Riccomini wrote:
>> > Ship It!
>
>Seems not to be committed yet?
>
>
>- Zhijie
>
>
>-----------------------------------------------------------
>This is an automatically generated e-mail. To reply, visit:
>https://reviews.apache.org/r/20514/#review41605
>-----------------------------------------------------------
>
>
>On April 26, 2014, 11:33 p.m., Zhijie Shen wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/20514/
>> -----------------------------------------------------------
>>
>> (Updated April 26, 2014, 11:33 p.m.)
>>
>>
>> Review request for samza.
>>
>>
>> Repository: samza
>>
>>
>> Description
>> -------
>>
>> SAMZA-218 Show container up-time in YARN AM
>>
>>
>> Diffs
>> -----
>>
>> build.gradle 65d95d4
>> gradle/dependency-versions.gradle 4338b23
>> samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml
>>6530bad
>>
>>samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.s
>>cala fa1642b
>>
>>samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskMan
>>ager.scala 58b2d30
>>
>>samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala
>>PRE-CREATION
>>
>>samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestSe
>>rvlet.scala 8fce8a7
>>
>> Diff: https://reviews.apache.org/r/20514/diff/
>>
>>
>> Testing
>> -------
>>
>>
>> Thanks,
>>
>> Zhijie Shen
>>
>>
>
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Zhijie Shen <zs...@hortonworks.com>.
> On April 28, 2014, 4:07 p.m., Chris Riccomini wrote:
> > Ship It!
Seems not to be committed yet?
- Zhijie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/#review41605
-----------------------------------------------------------
On April 26, 2014, 11:33 p.m., Zhijie Shen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20514/
> -----------------------------------------------------------
>
> (Updated April 26, 2014, 11:33 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-218 Show container up-time in YARN AM
>
>
> Diffs
> -----
>
> build.gradle 65d95d4
> gradle/dependency-versions.gradle 4338b23
> samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
> samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
>
> Diff: https://reviews.apache.org/r/20514/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhijie Shen
>
>
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Chris Riccomini <cr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/#review41605
-----------------------------------------------------------
Ship it!
Ship It!
- Chris Riccomini
On April 26, 2014, 11:33 p.m., Zhijie Shen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20514/
> -----------------------------------------------------------
>
> (Updated April 26, 2014, 11:33 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-218 Show container up-time in YARN AM
>
>
> Diffs
> -----
>
> build.gradle 65d95d4
> gradle/dependency-versions.gradle 4338b23
> samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
> samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
>
> Diff: https://reviews.apache.org/r/20514/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhijie Shen
>
>
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Zhijie Shen <zs...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/
-----------------------------------------------------------
(Updated April 26, 2014, 11:33 p.m.)
Review request for samza.
Changes
-------
Since nobody objects the ISO formatter, I keep to it in the new patch. In addition, I open the option to format the time with the customized formatter. The new patch also takes care of the dependency issue.
Repository: samza
Description
-------
SAMZA-218 Show container up-time in YARN AM
Diffs
-----
build.gradle 65d95d4
gradle/dependency-versions.gradle 4338b23
samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
Diff: https://reviews.apache.org/r/20514/diff/
Testing
-------
Thanks,
Zhijie Shen
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Zhijie Shen <zs...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/
-----------------------------------------------------------
(Updated April 26, 2014, 11:33 p.m.)
Review request for samza.
Changes
-------
Since nobody objects the ISO formatter, I keep to it in the new patch. In addition, I open the option to format the time with the customized formatter. The new patch also takes care of the dependency issue.
Repository: samza
Description
-------
SAMZA-218 Show container up-time in YARN AM
Diffs (updated)
-----
build.gradle 65d95d4
gradle/dependency-versions.gradle 4338b23
samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
Diff: https://reviews.apache.org/r/20514/diff/
Testing
-------
Thanks,
Zhijie Shen
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Zhijie Shen <zs...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/
-----------------------------------------------------------
(Updated April 24, 2014, 7:11 a.m.)
Review request for samza.
Changes
-------
Upload a new patch according to Chris' comments
Repository: samza
Description
-------
SAMZA-218 Show container up-time in YARN AM
Diffs (updated)
-----
samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
Diff: https://reviews.apache.org/r/20514/diff/
Testing
-------
Thanks,
Zhijie Shen
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Zhijie Shen <zs...@hortonworks.com>.
> On April 21, 2014, 5:08 p.m., Chris Riccomini wrote:
> >
Hi Chris, thanks for reviewing this patch, and pointing out the availability of Joda, which is an amazing lib. I used to leverage it to implement the datetime type of Pig project:-) I simply choose the ISO format for the datetime (have timezone) and the duration, but I'm not sure the formats are friendly enough for people to read. Please let me know how you think about it. Anyway I upload a new patch to demonstrate the changes.
- Zhijie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/#review40889
-----------------------------------------------------------
On April 20, 2014, 10:48 p.m., Zhijie Shen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20514/
> -----------------------------------------------------------
>
> (Updated April 20, 2014, 10:48 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-218 Show container up-time in YARN AM
>
>
> Diffs
> -----
>
> samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
> samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
>
> Diff: https://reviews.apache.org/r/20514/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhijie Shen
>
>
Re: Review Request 20514: SAMZA-218 Show container up-time in YARN AM
Posted by Chris Riccomini <cr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20514/#review40889
-----------------------------------------------------------
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala
<https://reviews.apache.org/r/20514/#comment74139>
Would be good to have the time zone here as well.
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala
<https://reviews.apache.org/r/20514/#comment74138>
Since samza-yarn is transitively pulling in joda-time already (through Scalatra), we might as well make it an explicit compile-time dependency, and use its Duration, Period, and PeriodFormatter class here. This will let us eliminate all of the convertToUpTimeStr code.
samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala
<https://reviews.apache.org/r/20514/#comment74135>
Add an upTime here (like startTime).
samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala
<https://reviews.apache.org/r/20514/#comment74136>
start-time and up-time should be c.startTime and c.upTime, I think, since it's more generally useful than just the string.
- Chris Riccomini
On April 20, 2014, 10:48 p.m., Zhijie Shen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20514/
> -----------------------------------------------------------
>
> (Updated April 20, 2014, 10:48 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-218 Show container up-time in YARN AM
>
>
> Diffs
> -----
>
> samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 6530bad
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala fa1642b
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala 58b2d30
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/YarnContainer.scala PRE-CREATION
> samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala 8fce8a7
>
> Diff: https://reviews.apache.org/r/20514/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhijie Shen
>
>