----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36471/#review93785 -----------------------------------------------------------
As discussed, we definitely need a document on the auto-scaling design and config model. You can add a separate web-page to the site. Please make sure you create follow-up JIRAs for the same and include a auto-scaling config table as well. samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 30) <https://reviews.apache.org/r/36471/#comment148189> Can you mention the available modes/values for this config as a comment here? It's easier to understand rather than scrolling till the method where this is used. samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 31) <https://reviews.apache.org/r/36471/#comment148190> Mention a one-liner comment on what this config indicates. It can perhaps be renamed as "autoscaling.max.containers.allowed" One general comment: Try to keep the variable name similar to the configuration. For example, "autoscaling.max.containers.allowed" can be AUTOSCALING_MAX_CONTAINERS_ALLOWED. It might make the variable name really long. But it improved readability. samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 34) <https://reviews.apache.org/r/36471/#comment148191> Rename this to "autoscaling.analyzer.%s.num.datapoints" ? I can't think of anything better :) But let's not overuse "window" as it automatically makes you relate to "time" 1 line comment like - "number of data points the analyzer looks at" samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 35) <https://reviews.apache.org/r/36471/#comment148193> Does this represent the period for which we store the gathered metrics ? samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 36) <https://reviews.apache.org/r/36471/#comment148197> This is confusing because it only refers to the percentage of data-points above the trigger threshold. Try "autoscaling.analyzer.%s.percent.datapoints.above.trigger" or "autoscaling.analyzer.%s.datatpoints.above.trigger" ?? samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 37) <https://reviews.apache.org/r/36471/#comment148195> This is fine! Just add a one-liner comment here samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 38) <https://reviews.apache.org/r/36471/#comment148198> I am a little lost on this config even after reading the description below. I will sync up with you in person! samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 41) <https://reviews.apache.org/r/36471/#comment148199> The varaible name is fine. You can change the config name to look similar. "autoscaling.optimizer.max.allocatable.capacity" or does "autoscaling.optimizer.max.container.capacity" sound better?? samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java (line 42) <https://reviews.apache.org/r/36471/#comment148200> "remove" sounds too harsh. It should indicate how much memory to free up per container that you scale up?? Does "autoscaling.optimizer.percent.memory.revoke" or "autoscaling.optimizer.memory.revoke" sound correct? - Navina Ramesh On July 31, 2015, 4:24 a.m., Shadi A. Noghabi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36471/ > ----------------------------------------------------------- > > (Updated July 31, 2015, 4:24 a.m.) > > > Review request for samza and Navina Ramesh. > > > Repository: samza > > > Description > ------- > > This work is for SAMZA-719. Currently, a fixed number of containers is > assigned to a job as an input configuration parameter. However, with this > design jobs can fail due to lack of enough resources (such as memory), or > they can become a bottleneck in a workflow containing many jobs. While > auto-scaling is much broader term, the goal of this project will be to enable > a Samza job to automatically scale its containers such that there is improved > job performance. > > Based on the design, we need a profiler, analyzer, optimizer and deployer > module. > > -currently, all components added based on memory usage and the loop seems to > work in inital test > > -tests not added for all components, and further testing is needed. > > > Diffs > ----- > > build.gradle 0852adc4e8e0c2816afd1ebf433f1af6b44852f7 > checkstyle/import-control.xml 6654319392929857bb861d77763afd8a5ea7674c > gradle/dependency-versions.gradle fb06e8ed393d1a38abfa1a48fe5244fc7f6c7339 > > samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingMode.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/AutoScalingSystem.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/analyzer/Analyzer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/analyzer/MemoryAnalyzer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/deployer/ConfigManager.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/deployer/Deployer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/deployer/RestartingJobDeployer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/metrics/MemoryMetrics.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/metrics/MetricsHistory.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/metrics/SlidingWindowMetric.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/optimizer/CapacityBasedJobModel.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/optimizer/Optimizer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/optimizer/PerContainerMemoryOptimizer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/optimizer/PerContainerOptimizer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/profiler/Profiler.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/profiler/SnapshotReporterProfiler.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/autoScaling/stream/AutoScalingMetricsSystemConsumer.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java > b1078bdf7bddd16c9ccc6559b9efd40ca5ae67bc > > samza-core/src/main/scala/org/apache/samza/autoScaling/AutoScalingCommandLine.scala > PRE-CREATION > > samza-core/src/main/scala/org/apache/samza/autoScaling/stream/AutoScalingMetricsSystemFactory.scala > PRE-CREATION > > samza-core/src/main/scala/org/apache/samza/config/AutoScalingConfigRewriter.scala > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala > e4b14f4da6649eb78753ba3b3f529373b6f2dbe4 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 27b2517048ad5730762506426ee7578c66181db8 > > samza-core/src/main/scala/org/apache/samza/container/grouper/task/GroupByContainerCountFactory.scala > eca6215a1b84c6dc0a4aa1e9e0f8d9f8bff1b467 > > samza-core/src/main/scala/org/apache/samza/container/grouper/task/GroupByJobModel.scala > PRE-CREATION > > samza-core/src/main/scala/org/apache/samza/container/grouper/task/GroupByjobModelFactory.scala > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala > 0dbf14b8bbb8236a8cdbe037a99dec91c5e63965 > samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala > 1c178a661e449c6bdfc4ce431aef9bb2d261a6c2 > samza-core/src/main/scala/org/apache/samza/metrics/JvmMetrics.scala > 64daa0f0bb7c6350a57e06705aef41b781779416 > samza-core/src/main/scala/org/apache/samza/util/Util.scala > 419452c0d48d8faa84279fcf02a03e87309821d8 > > samza-core/src/test/java/org/apache/samza/autoScaling/AutoScalingConfigTest.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/autoScaling/SlidingWindowMetricTest.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/autoScaling/analyzer/MockAnalyzer.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/autoScaling/optimizer/CapacityBasedJobModelTest.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/autoScaling/optimizer/MemoryOptimizerTest.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/autoScaling/optimizer/PerContainerMemoryOptimizerTest.java > PRE-CREATION > > samza-core/src/test/scala/org/apache/samza/container/grouper/task/TestGroupByJobModel.scala > PRE-CREATION > samza-shell/src/main/bash/run-auto-scaling.sh PRE-CREATION > 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/SamzaAppMaster.scala > af42c6a6636953a95f79837fe372e0dbd735df70 > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMaster.scala > 7b7d86a43c69e72c47eaa91f68be24e0f4022891 > > Diff: https://reviews.apache.org/r/36471/diff/ > > > Testing > ------- > > tested with hello samza > > > Thanks, > > Shadi A. Noghabi > >
