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

Ship it!


LGTM. Just two nits. Thanks!


samza-autoscaling/src/main/java/org/apache/samza/autoscaling/utils/YarnUtil.java
 (line 70)
<https://reviews.apache.org/r/36006/#comment149321>

    Question: why is this function only limited to running application 
instances? It seems maybe getRunningAppId() is more accurate?


- Yi Pan (Data Infrastructure)


On Aug. 8, 2015, 6:19 p.m., Shadi A. Noghabi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36006/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2015, 6:19 p.m.)
> 
> 
> Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and 
> Naveen Somasundaram.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> This RBis for SAMZA-724.
> 
> After a job is submitted, it might need some configuration change, 
> specifically it might need more containers. In SAMZA-704 a tool is being 
> added to write to the coordinator stream (CoordinatorStreamWriter).  This 
> tool can be used to write new configurations to the coordinator stream. 
> However, another tool (ConfigManager) is needed to read the config changes 
> and react to them, which is the goal of this task. This tool should be 
> brought up after the job is submitted and read any config changes added to 
> the coordinator stream, and react to each accordingly. 
> 
> This tool, called the Config Manager, is focusing on handling container 
> changs by reacting to set-config massages with key "yarn.container.count". 
> 
> The config manager is a separate standa alone module, that should be brought 
> up separately after the submission of a job. Therefore, you have to add two 
> configurations to the input config file:
> 1. yarn.rm.address= <ip of resource manager in yarn. ex: localhost >
> 2. yarn.rm.port= <the port of the resource manager http server. ex: 8088 >
> 
> The config manger will periodically poll the coordinator stream to see if 
> there are any new messages. This period is set to 100 ms by deafualt. 
> However, it can be configured by adding 
> configManager.polling.interval=<polling interval> to the input config file. 
> Thus, overal the command to run the config manager along with the job would 
> be:
> 
> 
> <path to samza deployment>/bin/run-config-manager.sh --config-factory=<config 
> factory> --config-path=<path to config file of a job>
> 
> 
> Diffs
> -----
> 
>   build.gradle 0852adc4e8e0c2816afd1ebf433f1af6b44852f7 
>   checkstyle/import-control.xml 24ed680785175f3cdf955602b7a813684edd813e 
>   gradle/dependency-versions.gradle fb06e8ed393d1a38abfa1a48fe5244fc7f6c7339 
>   
> samza-autoscaling/src/main/java/org/apache/samza/autoscaling/deployer/ConfigManager.java
>  PRE-CREATION 
>   
> samza-autoscaling/src/main/java/org/apache/samza/autoscaling/utils/YarnUtil.java
>  PRE-CREATION 
>   
> samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java
>  2277a732b9ab763edf19a0fbec288ff72b27583b 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 
> d7c928c7401e539a370d4e82276e7dabbce1b638 
>   samza-shell/src/main/bash/run-config-manager.sh PRE-CREATION 
>   
> samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala
>  ea702a919348305ff95ce0b4ca1996a13aff04ec 
>   
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterService.scala
>  ce88698c12c4bf6f4cf128f92d60b0b9496997d7 
>   settings.gradle 19bff971ad221084dac10d3f7f3facfa42b829a7 
> 
> Diff: https://reviews.apache.org/r/36006/diff/
> 
> 
> Testing
> -------
> 
> Tested with hello samza and works properly.
> 
> 
> Thanks,
> 
> Shadi A. Noghabi
> 
>

Reply via email to