----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53809/#review156144 -----------------------------------------------------------
Ship it! Ship It! - Sid Wagle On Nov. 16, 2016, 11:17 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53809/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2016, 11:17 p.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 > > ambari-server/src/test/java/org/apache/ambari/server/configuration/RecoveryConfigHelperTest.java > 6533e1c > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java > e54a117 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > 0fdaa46 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderHDP22Test.java > 96810cf > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > 14e3d08 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ComponentVersionCheckActionTest.java > 0163024 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java > 7ab2856 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsersTest.java > 314e955 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeActionTest.java > 4c1d7a3 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathActionTest.java > 9bde631 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigActionTest.java > 907194c > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/KerberosKeytabsActionTest.java > d374d75 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculationTest.java > e673714 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculationTest.java > 25acb45 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfigTest.java > e65a824 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/UpgradeActionTest.java > 8f9d4f4 > > ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java > 80665a5 > > ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java > d50c92d > > ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertReceivedListenerTest.java > 1867bda > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterDeadlockTest.java > 4fdcc22 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java > 90a3d02 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java > 5886234 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ServiceComponentHostConcurrentWriteDeadlockTest.java > 1f09002 > > ambari-server/src/test/java/org/apache/ambari/server/state/host/HostTest.java > 596f381 > > ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java > 14a8de6 > > ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java > ee64ac9 > > ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java > 29f40fb > > Diff: https://reviews.apache.org/r/53809/diff/ > > > Testing > ------- > > Tests run: 4768, Failures: 0, Errors: 0, Skipped: 42 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 25:26 min > [INFO] Finished at: 2016-11-16T17:19:45-05:00 > [INFO] Final Memory: 54M/791M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Jonathan Hurley > >
