You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Robert Nettleton <rn...@hortonworks.com> on 2015/05/12 22:55:39 UTC

Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

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

Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.


Bugs: AMBARI-11087
    https://issues.apache.org/jira/browse/AMBARI-11087


Repository: ambari


Description
-------

This patch resolves AMBARI-11087.

The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 

This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 

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


Testing
-------

1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
2. Ran the ambari-server unit tests, ran into 1 un-related failure:
"Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.


Thanks,

Robert Nettleton


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34123/#review83479
-----------------------------------------------------------

Ship it!


Ship It!

- Robert Levas


On May 12, 2015, 4:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 4:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On May 12, 2015, 9:31 p.m., John Speidel wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, line 1396
> > <https://reviews.apache.org/r/34123/diff/1/?file=956806#file956806line1396>
> >
> >     what about double quoted style?
> 
> Robert Nettleton wrote:
>     Double-quoted flow style was not necessary for this particular patch.  I've added the enum for the sake of completeness, but did not want to add this unless we needed it for config processing.
> 
> John Speidel wrote:
>     Seems dangerous to me to have the double quote flow available but not implemented.  I can certainly see somebody utilizing it in the future because it is available via the enum and not checking to see if it is actually implemented.  If you don't want to implement it, it would be safer to remove it from the enum.

It's important to note that this is an internal implementation class for the BlueprintConfigurationProcessor, and not a public-facing API.  While I don't agree that this is dangerous, I'll remove the enum for double quoted flow style in order to resolve this issue.  Once I remove the enum, I'll resubmit the patch.


- Robert


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


On May 12, 2015, 8:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 8:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On May 12, 2015, 9:31 p.m., John Speidel wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, line 1396
> > <https://reviews.apache.org/r/34123/diff/1/?file=956806#file956806line1396>
> >
> >     what about double quoted style?

Double-quoted flow style was not necessary for this particular patch.  I've added the enum for the sake of completeness, but did not want to add this unless we needed it for config processing.


- Robert


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


On May 12, 2015, 8:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 8:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On May 12, 2015, 9:31 p.m., John Speidel wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, line 1396
> > <https://reviews.apache.org/r/34123/diff/1/?file=956806#file956806line1396>
> >
> >     what about double quoted style?
> 
> Robert Nettleton wrote:
>     Double-quoted flow style was not necessary for this particular patch.  I've added the enum for the sake of completeness, but did not want to add this unless we needed it for config processing.
> 
> John Speidel wrote:
>     Seems dangerous to me to have the double quote flow available but not implemented.  I can certainly see somebody utilizing it in the future because it is available via the enum and not checking to see if it is actually implemented.  If you don't want to implement it, it would be safer to remove it from the enum.
> 
> Robert Nettleton wrote:
>     It's important to note that this is an internal implementation class for the BlueprintConfigurationProcessor, and not a public-facing API.  While I don't agree that this is dangerous, I'll remove the enum for double quoted flow style in order to resolve this issue.  Once I remove the enum, I'll resubmit the patch.

Fixed in version 2 of this patch.


- Robert


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


On May 13, 2015, 3:09 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 13, 2015, 3:09 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by John Speidel <js...@hortonworks.com>.

> On May 12, 2015, 9:31 p.m., John Speidel wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, line 1396
> > <https://reviews.apache.org/r/34123/diff/1/?file=956806#file956806line1396>
> >
> >     what about double quoted style?
> 
> Robert Nettleton wrote:
>     Double-quoted flow style was not necessary for this particular patch.  I've added the enum for the sake of completeness, but did not want to add this unless we needed it for config processing.

Seems dangerous to me to have the double quote flow available but not implemented.  I can certainly see somebody utilizing it in the future because it is available via the enum and not checking to see if it is actually implemented.  If you don't want to implement it, it would be safer to remove it from the enum.


- John


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


On May 12, 2015, 8:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 8:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34123/#review83489
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
<https://reviews.apache.org/r/34123/#comment134495>

    what about double quoted style?


- John Speidel


On May 12, 2015, 8:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 8:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34123/#review83614
-----------------------------------------------------------

Ship it!


Ship It!

- John Speidel


On May 13, 2015, 3:09 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34123/
> -----------------------------------------------------------
> 
> (Updated May 13, 2015, 3:09 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.
> 
> 
> Bugs: AMBARI-11087
>     https://issues.apache.org/jira/browse/AMBARI-11087
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-11087.
> 
> The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 
> 
> This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 
> 
> Diff: https://reviews.apache.org/r/34123/diff/
> 
> 
> Testing
> -------
> 
> 1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
> 2. Ran the ambari-server unit tests, ran into 1 un-related failure:
> "Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 34123: Add support for deploying Storm Nimbus HA clusters with Blueprints

Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34123/
-----------------------------------------------------------

(Updated May 13, 2015, 3:09 p.m.)


Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas.


Changes
-------

Uploaded version 2 of this patch, to address reviewer comments.


Bugs: AMBARI-11087
    https://issues.apache.org/jira/browse/AMBARI-11087


Repository: ambari


Description
-------

This patch resolves AMBARI-11087.

The BlueprintConfigurationProcessor has been modified to account for a new Storm property, "nimbus.seeds", that must be set to a comma-separated list of Nimbus Server hosts.  This property is used by Storm clients to discover the list of HA Nimbus servers available in the cluster.  This property uses Yaml, and so the default Yaml PropertyUpdater in the BlueprintConfigurationProcessor has been modified to account for different Yaml Flow styles.  This patch also registers this property updater to "nimbus.seeds" for processing during cluster creation. 

This patch also adds unit tests to verify this change, and updates existing unit tests to accomodate this change as well.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7938cc1 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java e7b0c64 

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


Testing
-------

1. Manual testing done with a 3-node cluster.  Current problems with Storm keep the initial Storm services from starting properly, but I was able to verify that the "nimbus.seeds" configuration property is set correctly at the completion of the cluster startup.  With a local patch to Storm itself, I can get all the Storm services to start properly. 
2. Ran the ambari-server unit tests, ran into 1 un-related failure:
"Failed tests:   testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>".  This unit test failure will be fixed in a separate patch by the Metrics team.


Thanks,

Robert Nettleton