> On Nov. 18, 2016, 11:21 p.m., Sid Wagle wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java, > > line 402 > > <https://reviews.apache.org/r/53881/diff/2/?file=1567042#file1567042line402> > > > > private modifier will not allow Transaction annotations from getting > > chained. > > Jonathan Hurley wrote: > Really ... I thought that the way in which we bind @Transactional allows > us to get around this. With that said, I'm curious why @Transactional doesn't > warn on this (or we don't have a test for it). > > `# grep -r --include "*.java" "@Transactional" src/main -A1 | grep > private | wc -l` > `14` > > So we have 14 instances of using @Transactional with private methods.
We had scrubbed all these in 2.2.2, quite possible these have re-appeared. Yes we need to have an unit test for this, I remember discussing this with Myroslav P on adding a test, I will open a Jira for it. - Sid ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53881/#review156352 ----------------------------------------------------------- On Nov. 18, 2016, 11:37 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53881/ > ----------------------------------------------------------- > > (Updated Nov. 18, 2016, 11:37 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Nate Cole, Robert Levas, and > Sid Wagle. > > > Bugs: AMBARI-18933 > https://issues.apache.org/jira/browse/AMBARI-18933 > > > Repository: ambari > > > Description > ------- > > {{ConfigGroupImpl}} 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. > > {{ConfigGroupImpl}} should be changed to: > - No longer store entity information > - No longer require locks around primitive state > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java > b957f0a > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java > e68839f > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 8b157c7 > > ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java > 1b29c9b > > ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java > 9abadf3 > > ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java > 9a2fc88 > > ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java > ed43ee1 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > 098efa9 > > ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java > 75853db > > ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java > a3a7e11 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java > 69cfc9f > > ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java > 8db5190 > > ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java > 68a8d4c > > Diff: https://reviews.apache.org/r/53881/diff/ > > > Testing > ------- > > Tests run: 4772, Failures: 0, Errors: 0, Skipped: 37 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 29:17 min > [INFO] Finished at: 2016-11-18T14:02:44-05:00 > [INFO] Final Memory: 57M/731M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Jonathan Hurley > >
