> On Травень 8, 2015, 8:54 після полудня, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java, > > line 124 > > <https://reviews.apache.org/r/33663/diff/5/?file=953065#file953065line124> > > > > I'm very worried about this pattern. If you did actually fix this bug > > using @RequireSession, it's way too easy to miss for anyone adding new > > methods. You've also created an interceptor on methods that get called a > > lot in larger clusters potentially leading to a bottleneck. > > > > Looking at AmbariLocalSessionInterceptor, it's opening a brand new unit > > of work every time this method is invoked and then closing that unit of > > work. Seems like an anti-pattern to me; we're not really supposed to be > > doing that on a per-call basis. Instead, it seems like what needs to be > > done is have the unit of work opened for the thread so that it can do all > > of its work (1 to n) queries and then close the unit of work when it's done. > > > > Basically, what we have here looks analogous to method-level locking > > when what you really need is a higher-level lock. Any call into these > > methods from within the context of a request will have the unit of work > > already. Anything that does not will need to start and end its own unit of > > work. > > > > Although this approach might work, it's not very performant and is > > still prone to errors when adding new methods. > > Jonathan Hurley wrote: > +1 with the condition that we open a new Jira to track moving away from > this pattern and possible using EntityManagerFactory in threads outside of > the jetty container.
New unit of work is created for top-level anotaion only, nested ones do nothing. And in terms of Guice-persist: creating of UoW is nothing more than new EntityManager(). We never hit performance issues with annotations, with current pattern we have single session (UoW) per request, and for each ActionScheduler iteration. We can do same for event processing by starting single session for each event. - Myroslav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33663/#review83030 ----------------------------------------------------------- On Травень 8, 2015, 5:10 після полудня, Dmitro Lisnichenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33663/ > ----------------------------------------------------------- > > (Updated Травень 8, 2015, 5:10 після полудня) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav > Papirkovskyy, and Nate Cole. > > > Bugs: AMBARI-10818 > https://issues.apache.org/jira/browse/AMBARI-10818 > > > Repository: ambari > > > Description > ------- > > Problem here is that DistributeRepositoriesActionListener is executed in a > separate thread. So we have to use UnitOfWork just like at > org.apache.ambari.server.actionmanager.ActionScheduler#doWork , otherwise > EntityManager cache is not updated on DB updates. I mean that > RepositoryVersion state at DB is INSTALLING, and API shows INSTALLING, but > RepositoryVersion state in DistributeRepositoriesActionListener is still > INSTALLED, and cluster state transition is not performed. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/upgrade/DistributeRepositoriesActionListener.java > 21e3a48 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java > d838d25 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java > 85973b1 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java > 9a2be15 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ClusterDAO.java > d3c4bd4 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/GroupDAO.java > bd5defb > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > 119737c > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/MemberDAO.java > e831db2 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PermissionDAO.java > 939c32b > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrincipalDAO.java > 7ac4f05 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrincipalTypeDAO.java > 046345a > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrivilegeDAO.java > 4fda7bc > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java > d55a00f > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RequestDAO.java > 7e190a3 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StackDAO.java > 6405068 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java > c2b551b > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/TopologyHostTaskDAO.java > 031601a > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java > a9b913f > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java > 2fcb087 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ViewDAO.java > bbbab63 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ViewInstanceDAO.java > 91a2e72 > > ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java > 5be6e44 > > Diff: https://reviews.apache.org/r/33663/diff/ > > > Testing > ------- > > Same tests are failing on trunk > > Tests in error: > > test220Cardinality(org.apache.ambari.server.api.services.KerberosServiceMetaInfoTest): > Guice provision errors:(..) > > test220AutoDeploy(org.apache.ambari.server.api.services.KerberosServiceMetaInfoTest): > Guice provision errors:(..) > > test220Dependencies(org.apache.ambari.server.api.services.KerberosServiceMetaInfoTest): > Guice provision errors:(..) > > testCommonOozieServiceDescriptor(org.apache.ambari.server.stack.KerberosDescriptorTest): > > /media/plextor/review_ambari/ambari-server/target/classes/common-services/OOZIE/5.0.0.2.3/kerberos.json > is not a readable file > > Tests run: 2951, Failures: 0, Errors: 4, Skipped: 17 > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Ambari Views ...................................... SUCCESS [2.900s] > [INFO] Ambari Metrics Common ............................. SUCCESS [1.570s] > [INFO] Ambari Server ..................................... FAILURE > [43:59.418s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD FAILURE > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 44:04.603s > [INFO] Finished at: Wed May 06 20:25:17 EEST 2015 > [INFO] Final Memory: 32M/268M > [INFO] ---------------------------------------------------------------------- > > > Thanks, > > Dmitro Lisnichenko > >
