ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Nettleton" <rnettle...@hortonworks.com>
Subject Re: Review Request 27006: Adding handler code for components with 0-1 cardinality in Blueprint configuration processor
Date Wed, 22 Oct 2014 14:59:36 GMT


> On Oct. 22, 2014, 12:46 p.m., Nate Cole wrote:
> >

Thanks for the review!


> On Oct. 22, 2014, 12:46 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Cardinality.java,
line 40
> > <https://reviews.apache.org/r/27006/diff/1/?file=728468#file728468line40>
> >
> >     Never understood why * was never used, but ok.

I'm not exactly sure, since I've basically moved this existing code to a top-level class,
and have left the impl unchanged.   Since this cardinality value is used in the stacks, it
might be difficult to change without breaking things.


> On Oct. 22, 2014, 12:46 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java,
lines 40-47
> > <https://reviews.apache.org/r/27006/diff/1/?file=728470#file728470line40>
> >
> >     We have a lot of Stack representations already.  Can one of them be augmented?

It's possible, but this was the only Stack parsing class I could find that included support
for Cardinality.  In the future, I'm definitely willing to use a more general class if it
becomes available, but would rather stick with this one for the 1.7.0 timeframe.  This particular
class has the advantage of handling all the details of the Stack query API, so I find it a
bit simpler to use.


- Robert


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


On Oct. 22, 2014, 12:57 a.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27006/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2014, 12:57 a.m.)
> 
> 
> Review request for Ambari, John Speidel and Nate Cole.
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch implements a fix for bug: AMBARI-7738.
> 
> The Blueprints configuration processor was not properly handling
>   the case of an optional service in an HDP 2.1 stack deployment.
> 
> The problem occurs when the Yarn "APP_TIMELINE_SERVER" is not
>   included in a Blueprint that references HDP 2.1.  Posting the
>   Blueprint will work properly, but launching a cluster based
>   on this Blueprint will fail, with an error mentioning that
>   the APP_TIMELINE_SERVER is not included.
> 
> The APP_TIMELINE_SERVER has a cardinality of "0-1" for
>   HDP 2.1, meaning that 0 or 1 instances of this service
>   can be deployed.  This cardinality was selected for HDP
>   2.1 because this service was considered a technical
>   preview for the HDP 2.1 stack.
> 
> While this particular Yarn component demonstrates the problem,
>   the issue is actually a bit more generic. The Blueprint
>   config processor is not currently aware of the cardinality
>   of each service being deployed, so the failure occurs when the
>   processor attempts to update configuration for a serivce that
>   doesn't exist in the proposed cluster.
> 
> This patch addresses the problem with the following changes:
> 
>  - Moves two inner classes (Stack, Cardinality) to the
>      top-level, as package-level classes.  These classes
>      offer useful processing features for the Blueprint
>      config processor, and are more useful as standalone
>      classes.
> 
>  - Modifies the signatures of the PropertyUpdater interface,
>    any implementations of this interface, and the call stack
>    from the ClusterResourceProvider to the BlueprintConfiguration
>    processor, such that the parsed Stack object is available
>    to the PropertyUpdater instances.
> 
>  - Modifies the implementation for the SingleHostTopologyUpdater
>    to query the Stack definition in the event that no hosts
>    are found when trying to update a property.  If a cardinality
>    of 0 is found to be valid for that service, then the original
>    value for the property is returned.  If a cardinality of 0
>    is not valid for this service, then an IllegalArgumentException
>    is thrown back to the caller, indicating that an error has
>    occurred (this was the previous behavior in both cases).
> 
>  - Updates various existing unit tests to reflect this change.
> 
>  - Adds new unit tests to verify the fix.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseBlueprintProcessor.java
abd22a4 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
0b58c8d 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Cardinality.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java
090eb92 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseBlueprintProcessorTest.java
be5aea8 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
a57bacb 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java
c1185ae 
> 
> Diff: https://reviews.apache.org/r/27006/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran the ambari-server unit test suite (both trunk and 1.7.0), and all unit tests are
passing with this patch applied.
> 2. Manually verified the fix against the trunk code.
> 3. Manually verified the fix against the 1.7.0 code. 
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message