----------------------------------------------------------- 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 > >
