> On March 9, 2016, 9:19 p.m., Aravindan Vijayan wrote: > > ambari-metrics/ambari-metrics-timelineservice/conf/unix/ambari-metrics-collector, > > line 382 > > <https://reviews.apache.org/r/44525/diff/1/?file=1292291#file1292291line382> > > > > Quick question, why do we need a leading ":" in these statements?
Yes we need the leading : because that's what makes the statement a no-op. The actual action we are interested in is the side-effect which on line 376 is assigning AMS_HBASE_FIFO_COMPACTION_ENABLED=true if and only if AMS_HBASE_FIFO_COMPACTION_ENABLED is found to be unset. Similar for the other ones. The other option was to just omit these lines altogether but I think what I have in the change is a wee-bit better. - Shantanu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44525/#review122808 ----------------------------------------------------------- On March 9, 2016, 8:59 p.m., Shantanu Mundkur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44525/ > ----------------------------------------------------------- > > (Updated March 9, 2016, 8:59 p.m.) > > > Review request for Ambari, Aravindan Vijayan and Sid Wagle. > > > Bugs: AMBARI-15331 > https://issues.apache.org/jira/browse/AMBARI-15331 > > > Repository: ambari > > > Description > ------- > > AMBARI-15331: AMS HBase FIFO compaction policy and Normalizer settings are > not handled correctly > > A user could change the defaults for these settings via configuration updates > but at least two problems would result. > > 1) Updates to variables AMS_HBASE_FIFO_COMPACTION_ENABLED and > AMS_HBASE_NORMALIZER_ENABLED will not be honored. Same would be the case for > the recent addition to ams-env - AMS_HBASE_INIT_CHECK_ENABLED. > > These are hardcoded in ambari-metrics-collector rather than using what was > propagated via ams-env.sh.To fix we'll try to use the values we expect to be > determined by ams-env (which would also account for any user modification to > the configuration). To be conservative we'll use some default value (same as > it is today) but only if the variable was found not set. > > 2) During upgrade to Ambari 2.2.0 these settings will not be correctly > handled due to incorrect variable names and leads to exceptions e.g if > AMS_HBASE_FIFO_COMPACTION_ENABLED was expected to be set to 'false', for > instance if the AMS HBase version did not support FIFO compaction policy. > > HBASE_FIFO_COMPACTION_POLICY_ENABLED is used instead of > AMS_HBASE_FIFO_COMPACTION_ENABLED > HBASE_NORMALIZATION_ENABLED is used instead of AMS_HBASE_NORMALIZER_ENABLED > > Due to this and due to the hardcoded-values in ambari-metrics-collector > undesired behavior (collector not starting up, attempts to startup AMS > leaving hanging processes around) can result especially if the underlying AMS > HBase version does not support FIFO compaction policy. > > > Diffs > ----- > > > ambari-metrics/ambari-metrics-timelineservice/conf/unix/ambari-metrics-collector > 64a7848 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog220.java > ac6b3c5 > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/ams-env.xml > 78b8999 > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog220Test.java > 8263001 > > Diff: https://reviews.apache.org/r/44525/diff/ > > > Testing > ------- > > 1) Results : > > Tests run: 3930, Failures: 0, Errors: 0, Skipped: 33 > > 2) Updated UpgradeCatalog220Test testcase. > > 3) Manually tested: > > i) Set timeline.metrics.hbase.fifo.compaction.enabled to false and ensure it > was effective after deployment. AMS components were all started and > running successfully with dashboards and heatmaps being displayed as expected. > ii) Upgrade from Ambari 2.1.0 to Ambari 2.2.0 and ensured the property > timeline.metrics.hbase.fifo.compaction.enabled was false after deployment. > AMS components were all started and running successfully with dashboards and > heatmaps being displayed as expected. > iii) Updates to hbase.normalizer.enabled were effective in the configuration > for both the new install and upgrade cases. > > > File Attachments > ---------------- > > AMBARI-15331.patch > > https://reviews.apache.org/media/uploaded/files/2016/03/08/243b5224-b255-404a-bd24-1b3ce5f79649__AMBARI-15331.patch > > > Thanks, > > Shantanu Mundkur > >