You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by József Márton Jung <j....@levi9.com> on 2015/07/01 16:07:52 UTC

Review Request 36089: SAMZA-670 Allow easier access to JMX port

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36089/
-----------------------------------------------------------

Review request for samza.


Repository: samza


Description
-------

JMX address of application master and the containers is available through AM UI


Diffs
-----

  checkstyle/import-control.xml 3374f0c 
  samza-api/src/main/java/org/apache/samza/container/SamzaContainerContext.java fd7333b 
  samza-core/src/main/java/org/apache/samza/container/LocalityManager.java e661e12 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java 6c1e488 
  samza-core/src/main/java/org/apache/samza/job/model/ContainerModel.java 98a34bc 
  samza-core/src/main/java/org/apache/samza/job/model/JobModel.java 95a2ce5 
  samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala cbacd18 
  samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala 8ee034a 
  samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala f343faf 
  samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 9fb1aa9 
  samza-core/src/test/scala/org/apache/samza/container/TestTaskInstance.scala 7caad28 
  samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 1ce7d25 
  samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml cf0d2fc 
  samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala 20aa373 
  samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala 1445605 

Diff: https://reviews.apache.org/r/36089/diff/


Testing
-------


Thanks,

József Márton Jung


Re: Review Request 36089: SAMZA-670 Allow easier access to JMX port

Posted by József Márton Jung <j....@levi9.com>.

> On July 13, 2015, 9:44 p.m., Yan Fang wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, line 636
> > <https://reviews.apache.org/r/36089/diff/1/?file=996581#file996581line636>
> >
> >     I think a better way, which requires much fewer changes, is to call something like jmxServer.getJmxUrl, jmxServer.jmxTunelingUrl.
> >     
> >     jmxServer can be a variable of SamzaContainer Object.
> >     
> >     Then we do not need to change ContainerModel, JobModel, SamzaContext. Because there is no reason that we want to contain jmx information into those three objects.

The new patch fixes this issue. Also, whn I created the new patch, I merged the master branch back to my local SAMZA-670 branch, so the new diff can be cleanly applied to master.


- József Márton


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36089/#review91533
-----------------------------------------------------------


On July 14, 2015, 1:30 p.m., József Márton Jung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36089/
> -----------------------------------------------------------
> 
> (Updated July 14, 2015, 1:30 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> JMX address of application master and the containers is available through AM UI
> 
> 
> Diffs
> -----
> 
>   checkstyle/import-control.xml 3374f0c 
>   samza-core/src/main/java/org/apache/samza/container/LocalityManager.java e661e12 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java 6c1e488 
>   samza-core/src/main/java/org/apache/samza/job/model/JobModel.java 95a2ce5 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala cbacd18 
>   samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala 8ee034a 
>   samza-core/src/main/scala/org/apache/samza/job/local/ThreadJobFactory.scala 3f2f70e 
>   samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala f343faf 
>   samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 9fb1aa9 
>   samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml cf0d2fc 
>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala 20aa373 
>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala 1445605 
> 
> Diff: https://reviews.apache.org/r/36089/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> József Márton Jung
> 
>


Re: Review Request 36089: SAMZA-670 Allow easier access to JMX port

Posted by Yan Fang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36089/#review91533
-----------------------------------------------------------



samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (line 634)
<https://reviews.apache.org/r/36089/#comment144956>

    I think a better way, which requires much fewer changes, is to call something like jmxServer.getJmxUrl, jmxServer.jmxTunelingUrl.
    
    jmxServer can be a variable of SamzaContainer Object.
    
    Then we do not need to change ContainerModel, JobModel, SamzaContext. Because there is no reason that we want to contain jmx information into those three objects.


- Yan Fang


On July 1, 2015, 2:07 p.m., József Márton Jung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36089/
> -----------------------------------------------------------
> 
> (Updated July 1, 2015, 2:07 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> JMX address of application master and the containers is available through AM UI
> 
> 
> Diffs
> -----
> 
>   checkstyle/import-control.xml 3374f0c 
>   samza-api/src/main/java/org/apache/samza/container/SamzaContainerContext.java fd7333b 
>   samza-core/src/main/java/org/apache/samza/container/LocalityManager.java e661e12 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java 6c1e488 
>   samza-core/src/main/java/org/apache/samza/job/model/ContainerModel.java 98a34bc 
>   samza-core/src/main/java/org/apache/samza/job/model/JobModel.java 95a2ce5 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala cbacd18 
>   samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala 8ee034a 
>   samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala f343faf 
>   samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 9fb1aa9 
>   samza-core/src/test/scala/org/apache/samza/container/TestTaskInstance.scala 7caad28 
>   samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala 1ce7d25 
>   samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml cf0d2fc 
>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala 20aa373 
>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala 1445605 
> 
> Diff: https://reviews.apache.org/r/36089/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> József Márton Jung
> 
>


Re: Review Request 36089: SAMZA-670 Allow easier access to JMX port

Posted by József Márton Jung <j....@levi9.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36089/
-----------------------------------------------------------

(Updated July 14, 2015, 1:30 p.m.)


Review request for samza.


Repository: samza


Description
-------

JMX address of application master and the containers is available through AM UI


Diffs (updated)
-----

  checkstyle/import-control.xml 3374f0c 
  samza-core/src/main/java/org/apache/samza/container/LocalityManager.java e661e12 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java 6c1e488 
  samza-core/src/main/java/org/apache/samza/job/model/JobModel.java 95a2ce5 
  samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala cbacd18 
  samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala 8ee034a 
  samza-core/src/main/scala/org/apache/samza/job/local/ThreadJobFactory.scala 3f2f70e 
  samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala f343faf 
  samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 9fb1aa9 
  samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml cf0d2fc 
  samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMaster.scala 20aa373 
  samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala 1445605 

Diff: https://reviews.apache.org/r/36089/diff/


Testing
-------


Thanks,

József Márton Jung