Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ambari.apache.org
Content-Type: multipart/alternative;
 boundary="===============5542641612848055687=="
MIME-Version: 1.0
Subject: Re: Review Request 35251: Oozie crashes after start on blueprint
 install
From: "Robert Nettleton" <rnettleton@hortonworks.com>
To: "Vitalyi Brodetskyi" <vbrodetskyi@hortonworks.com>
Cc: "Andrew Onischuk" <aonishuk@hortonworks.com>,
 "Robert Nettleton" <rnettleton@hortonworks.com>,
 "Ambari" <dev@ambari.apache.org>
Date: Tue, 09 Jun 2015 13:42:50 -0000
Message-ID: <20150609134250.27735.92445@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Robert Nettleton" <noreply@reviews.apache.org>
References: <20150609130117.27736.56733@reviews.apache.org>
In-Reply-To: <20150609130117.27736.56733@reviews.apache.org>
Reply-To: "Robert Nettleton" <rnettleton@hortonworks.com>

--===============5542641612848055687==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


I don't think this patch should be merged.

The current BlueprintConfigurationProcessor could simply be updated by registering a new PropertyUpdater for the "oozie_heapsize" property.

While I applaud the concern for making something more general, its not clear to me that all configuration properties across the stack require the "m" suffix appended.  There may be other properties that require a MB unit of measure, but don't have the same requirements that the heapsize properties do.

At this stage in the release, I don't think this is an appropriate change to make.

I request that this change be modified to simply register a heapsize PropertyUpdater for the required properties.  

A more general solution should be implemented in the next release, likely with major changes to the stack definitions to support this.

- Robert Nettleton


On June 9, 2015, 1:01 p.m., Andrew Onischuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35251/
> -----------------------------------------------------------
> 
> (Updated June 9, 2015, 1:01 p.m.)
> 
> 
> Review request for Ambari and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-11811
>     https://issues.apache.org/jira/browse/AMBARI-11811
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This happens because some properties which show capacity don't have 'm'
> appended when installing via blueprints (via UI installs it is done by UI)
> this results in Oozie thinking that heapsize is in kb and failing with too low
> heapsize.
> 
> Also there are some other properties which don't have 'm' appended, but should
> have, which results in missconfiguration issues (instead on megabytes
> kilobytes are set)
> 
> The proposal on how to fix it is to dynamically find the properties with
> unit=MB and append 'm' to them, rather than hardcode that properties like it
> is done now.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java ca7b2b1 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ValueAttributesInfo.java 8054c54 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java eb583fd 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java a75cccb 
>   ambari-server/src/main/resources/common-services/OOZIE/4.0.0.2.0/configuration/oozie-env.xml 6439bc6 
>   ambari-server/src/main/resources/common-services/OOZIE/5.0.0.2.3/configuration/oozie-env.xml fe80bf5 
>   ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/metainfo.xml ca82d46 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 96abb8c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java b049fd3 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 64dfb28 
> 
> Diff: https://reviews.apache.org/r/35251/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Andrew Onischuk
> 
>


--===============5542641612848055687==--