----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53809/#review156075 -----------------------------------------------------------
Ship it! Ship It! - Nate Cole On Nov. 16, 2016, 8:43 a.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53809/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2016, 8:43 a.m.) > > > Review request for Ambari, Nate Cole, Robert Levas, Robert Nettleton, and Sid > Wagle. > > > Bugs: AMBARI-18906 > https://issues.apache.org/jira/browse/AMBARI-18906 > > > Repository: ambari > > > Description > ------- > > {{ConfigImpl}} uses internal locks around state which can lead to deadlocks > when this in called in the context of other business objects. In most cases, > this state-full data does not need locks placed around it. > > {{ConfigImpl}} should be changed to: > - No longer store entity information > - No longer require locks around primitive state > > One of the biggest changes was made to how configs are created. We used to > use this anti-pattern: > - Create config in-memory from factory (tracking persistence state) > - Set the tag on the config instance > - Persist the config > - Add config to cluster > > That was ridiculous - the factory should handle the creation of the entity > with the required data. In a few spots, I had caught bugs where we had > duplicated a ton of cluster entity logic when adding configs which was > missing some steps. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java > b04fdd7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java > 96bb8f9 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java > 5459ddb > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixLzoCodecPath.java > ffa21ab > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsers.java > 3a06476 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseConfigCalculation.java > 7f6d4b1 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeAction.java > b238bca > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathAction.java > 0e10160 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java > 0ade30b > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/OozieConfigCalculation.java > 4da67ca > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculation.java > ff4a20e > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculation.java > ba0da79 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfig.java > 299a373 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/YarnConfigCalculation.java > feefcaf > ambari-server/src/main/java/org/apache/ambari/server/state/Config.java > b35aad9 > > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigFactory.java > eaf68aa > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java > 28bcd5f > > ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java > 9917720 > > ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java > 83f8470 > > ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java > 6a8057c > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java > ffca51d > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java > 90a4421 > > ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatTestHelper.java > 43503fa > > ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java > 76ab45c > > Diff: https://reviews.apache.org/r/53809/diff/ > > > Testing > ------- > > The tests are a mess right now - about 280 compile errors. I'm working > through them. However, I don't think that should hold up feedback on the > review (especially since this is being done in a separate branch) > > > Thanks, > > Jonathan Hurley > >
