> On July 2, 2015, 3:27 p.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, > > lines 87-105 > > <https://reviews.apache.org/r/36133/diff/1/?file=998310#file998310line87> > > > > Its same code in all methods. Shall we move this to a private method - > > which takes required params and call from all methods? > > > > Also, seems like we are removing autocommit purposefully. Would > > autocommit give us any advantages? wrt performance? > > Deepak Barr wrote: > Setting autocommit false allows you to rollback on failures. This is > particularly useful in atomic transaction. But in our case, all db updates > are single statement only. So, autocommit=false is not useful for us. > Also,there are no advantages wrt performance as well in our case. > > Originally, the issue was a commit() call in > createFinishedQueriesTable(). Initially I thought that for hsqldb > connections, autocommit is false by default and thats why commit() call does > not throw any Exception. But later I found that all the connections created > from JDBC driver has autocommit=true. Its just that when you call commit() > when autocommit=true, HSQLDB driver does not throw an exception but MySQL > does. In other words, to fix the issue - the minimal change we need to do is > to remove the commit() call in createFinishedQueriesTable(). We can get rid > of the autocommit=false and rollback logics as well. > > Jaideep dhok wrote: > +1 Exception handling code can be refactored into a method.
Hey Jaideep/Amareshwari, Thank you for your feedback. On the final note, please advise if I should keep the autocommit=false/rollback logic in the patch OR should I remove it. Like I had mentioned, autocommit=false/rollback logic good to have for atomic transactions, but offers no benefit for our single transaction db updates. - Deepak ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36133/#review90232 ----------------------------------------------------------- On July 2, 2015, 2:57 p.m., Deepak Barr wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36133/ > ----------------------------------------------------------- > > (Updated July 2, 2015, 2:57 p.m.) > > > Review request for lens, Jaideep dhok and Pranav Agarwal. > > > Bugs: LENS-639 > https://issues.apache.org/jira/browse/LENS-639 > > > Repository: lens > > > Description > ------- > > Patch for LENS-639. All database updates will be handled by commits & > rollbacks. > > > Diffs > ----- > > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java > 1904350 > > Diff: https://reviews.apache.org/r/36133/diff/ > > > Testing > ------- > > Yes > > > Thanks, > > Deepak Barr > >
