> On April 11, 2016, 8:36 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java, > > lines 701-708 > > <https://reviews.apache.org/r/45904/diff/3/?file=1337530#file1337530line701> > > > > This solution is still far from ideal. You'll be making something like > > 20,000 database queries and if even 1 fails, then the entire transaction is > > rolled back. > > > > This is why we went for a pure JPQL approach here; it was clean and > > didn't cause any weird problems. > > > > This also doesn't address the fact that nothing prevents future JPQL > > queries from causing a similar problem. > > > > Perhaps we need to consider a different approach. The issue was that > > MySQL was using temporary tables outside of a transaction. If we can solve > > that problem, we can leave the existing JPQL. > > Myroslav Papirkovskyy wrote: > This is not quite correct. Issue was that EclipseLink uses MySQL > temporary tables INSIDE of transactions. > Temp tables outside of transaction are supported even with GTID enabled. > > However disabling temp tables simply doesn't work. EclipseLink starts to > generate invalid queries for MySQL. And after quick look at sources it seems > that major modifications are required to fix this. > For MySQL temp tables are used to bypass limitations of subqueries in > delete/update statements.
Ah, that stinks. Yeah, my hope was that telling MySQL to not use temp tables would be the ticket. But if it starts forming bad queries, that's no good either. This is quite bad in that we really can't leverage JPQL anymore if we say we support GTID. I'm curious if running in the WARN mode would be a good balances. This way, GTID will be enabled, but some Ambari queries which violated GTID constraints can still be allowed to execute. The current approach seems very fragile and extremely hard to enforce moving forward. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45904/#review128100 ----------------------------------------------------------- On April 8, 2016, 4:56 p.m., Sid Wagle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45904/ > ----------------------------------------------------------- > > (Updated April 8, 2016, 4:56 p.m.) > > > Review request for Ambari, Jonathan Hurley, Myroslav Papirkovskyy, and Sumit > Mohanty. > > > Bugs: AMBARI-15774 > https://issues.apache.org/jira/browse/AMBARI-15774 > > > Repository: ambari > > > Description > ------- > > Approach: > > - Added member varaibles only when underlying DB columns exist > - Changed DELETE via implcit join query to find and delete > - MySQL temp tables are only used for UPDATE ALL and DELETE ALL, so SELECT > and DELETE is a safer approach > - Visually inspected all Entities under > org.apache.ambari.server.orm.entities.* for absence of this pattern > > Issue: Ambari upgrade is failing from 2.2.0 to 2.2.1.1 with below error: > > Error output from schema upgrade command: > Exception in thread "main" org.apache.ambari.server.AmbariException: > Exception [EclipseLink-4002] (Eclipse Persistence Services - > 2.5.2.v20140319-9ad6abd): > org.eclipse.persistence.exceptions.DatabaseException > Internal Exception: > com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Table > 'ambari.TL_alert_notice' doesn't exist > Error Code: 1146 > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java > a3befa6 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAO.java > 82fa48a > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java > 1f1aa45 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java > 781d4cf > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java > 604b00e > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertHistoryEntity.java > 03ffcde > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertNoticeEntity.java > ae7495d > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java > d611fe8 > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertDefinitionDAOTest.java > 36e75e7 > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java > 10f099e > > Diff: https://reviews.apache.org/r/45904/diff/ > > > Testing > ------- > > Manually verified on 1 node repro cluster. > > All alert unit test passed. > > mvn clean test -Dtest=Alert* -Drat.ignoreErrors -DfailIfNoTests=false > > > Thanks, > > Sid Wagle > >
