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




ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/LogFeederAMSClient.java
 (line 61)
<https://reviews.apache.org/r/51814/#comment215968>

    Shouldn't the constructor be modified for getting multiple hosts? not sure 
what this returns null for everything but looks like the author of the sink 
left this a TODO for later.



ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/solr/metrics/SolrAmsClient.java
 (line 49)
<https://reviews.apache.org/r/51814/#comment215969>

    Ditto here, if there is no such config we should add it.



ambari-metrics/ambari-metrics-common/src/main/java/org/apache/hadoop/metrics2/sink/timeline/AbstractTimelineMetricsSink.java
 (line 335)
<https://reviews.apache.org/r/51814/#comment215970>

    Null check seems redundant maybe reorder as StringUtil.isEmpty followed by 
trim.



ambari-metrics/ambari-metrics-common/src/main/java/org/apache/hadoop/metrics2/sink/timeline/AbstractTimelineMetricsSink.java
 (line 473)
<https://reviews.apache.org/r/51814/#comment215971>

    Effectively with this change user cannot configure separate port on 
different hosts. Ambari allows this by the means of Config groups. Is this 
change necessary / practical? I am not flagging this as an issue since 
technically user can change the port for both collectors if there is a conflict 
on one of the hosts but wanted to make sure we have a reason to seggregate this 
info.


- Sid Wagle


On Sept. 12, 2016, 4:09 p.m., Dmytro Sen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51814/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2016, 4:09 p.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan and Sid Wagle.
> 
> 
> Bugs: AMBARI-18362
>     https://issues.apache.org/jira/browse/AMBARI-18362
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Update sinks to read multiple collector hostnames from configs
> 
> 
> Diffs
> -----
> 
>   
> ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/LogFeederAMSClient.java
>  da61d83 
>   
> ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/solr/metrics/SolrAmsClient.java
>  85ea69d 
>   
> ambari-metrics/ambari-metrics-common/src/main/java/org/apache/hadoop/metrics2/sink/timeline/AbstractTimelineMetricsSink.java
>  426eb42 
>   
> ambari-metrics/ambari-metrics-common/src/test/java/org/apache/hadoop/metrics2/sink/timeline/availability/MetricCollectorHATest.java
>  7fadeb2 
>   
> ambari-metrics/ambari-metrics-common/src/test/java/org/apache/hadoop/metrics2/sink/timeline/cache/HandleConnectExceptionTest.java
>  ccaa574 
>   
> ambari-metrics/ambari-metrics-flume-sink/src/main/java/org/apache/hadoop/metrics2/sink/flume/FlumeTimelineMetricsSink.java
>  1b36e9a 
>   
> ambari-metrics/ambari-metrics-hadoop-sink/src/main/java/org/apache/hadoop/metrics2/sink/timeline/HadoopTimelineMetricsSink.java
>  c534121 
>   
> ambari-metrics/ambari-metrics-hadoop-sink/src/test/java/org/apache/hadoop/metrics2/sink/timeline/HadoopTimelineMetricsSinkTest.java
>  ea7f72d 
>   
> ambari-metrics/ambari-metrics-kafka-sink/src/main/java/org/apache/hadoop/metrics2/sink/kafka/KafkaTimelineMetricsReporter.java
>  9c16564 
>   
> ambari-metrics/ambari-metrics-storm-sink-legacy/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsReporter.java
>  73381d9 
>   
> ambari-metrics/ambari-metrics-storm-sink-legacy/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSink.java
>  0d3b770 
>   
> ambari-metrics/ambari-metrics-storm-sink/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsReporter.java
>  9082e70 
>   
> ambari-metrics/ambari-metrics-storm-sink/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSink.java
>  f6531c8 
>   
> ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java
>  4618618 
>   
> ambari-server/src/main/resources/common-services/ACCUMULO/1.6.1.2.2.0/package/scripts/params.py
>  4c111f1 
>   
> ambari-server/src/main/resources/common-services/ACCUMULO/1.6.1.2.2.0/package/templates/hadoop-metrics2-accumulo.properties.j2
>  3cd535e 
>   
> ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/templates/hadoop-metrics2-hbase.properties.j2
>  586cd47 
>   
> ambari-server/src/main/resources/common-services/FLUME/1.4.0.2.0/package/scripts/flume.py
>  b839eb8 
>   
> ambari-server/src/main/resources/common-services/FLUME/1.4.0.2.0/package/scripts/params.py
>  d3a9294 
>   
> ambari-server/src/main/resources/common-services/FLUME/1.4.0.2.0/package/templates/flume-metrics2.properties.j2
>  b960296 
>   
> ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/scripts/params_linux.py
>  49b684b 
>   
> ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/templates/hadoop-metrics2-hbase.properties-GANGLIA-MASTER.j2
>  e62ce9e 
>   
> ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/templates/hadoop-metrics2-hbase.properties-GANGLIA-RS.j2
>  8183333 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/params_linux.py
>  56e3ce6 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/templates/hadoop-metrics2-hivemetastore.properties.j2
>  88be81b 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/templates/hadoop-metrics2-hiveserver2.properties.j2
>  3d71867 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/templates/hadoop-metrics2-llapdaemon.j2
>  ba44af5 
>   
> ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/templates/hadoop-metrics2-llaptaskscheduler.j2
>  77f128b 
>   
> ambari-server/src/main/resources/common-services/KAFKA/0.8.1/configuration/kafka-broker.xml
>  ab4d701 
>   
> ambari-server/src/main/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  6cc85f4 
>   
> ambari-server/src/main/resources/common-services/KAFKA/0.8.1/package/scripts/params.py
>  f631ac9 
>   
> ambari-server/src/main/resources/common-services/STORM/0.9.1/package/scripts/params_linux.py
>  f96aeeb 
>   
> ambari-server/src/main/resources/common-services/STORM/0.9.1/package/templates/config.yaml.j2
>  a9760cb 
>   
> ambari-server/src/main/resources/common-services/STORM/0.9.1/package/templates/storm-metrics2.properties.j2
>  1f0875f 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/hooks/before-START/scripts/params.py
>  45eab2f 
>   
> ambari-server/src/main/resources/stacks/HDP/2.0.6/hooks/before-START/templates/hadoop-metrics2.properties.j2
>  374795f 
>   ambari-server/src/main/resources/stacks/HDP/2.0.6/services/stack_advisor.py 
> fd0dfed 
> 
> Diff: https://reviews.apache.org/r/51814/diff/
> 
> 
> Testing
> -------
> 
> Tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Reply via email to