> On Aug. 4, 2016, 7:03 p.m., Puneet Gupta wrote: > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java, > > line 1041 > > <https://reviews.apache.org/r/50600/diff/6/?file=1461126#file1461126line1041> > > > > will driver status cancelled be different from query status cancelled ?
Removed duplication. Now boolean is not there, only enum check everywhere. > On Aug. 4, 2016, 7:03 p.m., Puneet Gupta wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/QueryDriverStatusUpdateListener.java, > > line 27 > > <https://reviews.apache.org/r/50600/diff/6/?file=1461134#file1461134line27> > > > > Should we name it DriverQueryStatusUpdateListener That would sound like a listener for driver's status updates instead of a listener for query's driver-states-updates. > On Aug. 4, 2016, 7:03 p.m., Puneet Gupta wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java, > > line 495 > > <https://reviews.apache.org/r/50600/diff/6/?file=1461137#file1461137line495> > > > > We dont need this anymore? This method was unused. > On Aug. 4, 2016, 7:03 p.m., Puneet Gupta wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 820 > > <https://reviews.apache.org/r/50600/diff/6/?file=1461140#file1461140line820> > > > > Should we Remove Async from name ? There's already a StatusPoller. The name `StatusUpdater` would be confusing. Hence added `Async`. > On Aug. 4, 2016, 7:03 p.m., Puneet Gupta wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 825 > > <https://reviews.apache.org/r/50600/diff/6/?file=1461140#file1461140line825> > > > > Should we optimize in this case ? updateState() will again ask the > > driver for status though driver already sent the updated status Optimized by adding a boolean and overriding the method. > On Aug. 4, 2016, 7:03 p.m., Puneet Gupta wrote: > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java, > > line 993 > > <https://reviews.apache.org/r/50600/diff/6/?file=1461126#file1461126line993> > > > > should we remove registerForCompletionNotification method from drivers > > and rely on QueryContext.registerStatusUpdateListener()? > > > > Let notification be realtime only for PUSH type drivers and let PULL > > type drivers rely on status poller to update status and trigger > > notifictaion (we ll be removing the extra polling thread from Hive in this > > case.. don't think its useful anyway for hive queries) This change might change the functionality of `execute with timeout` for hive queries. Didn't want to change that. We can certainly pick it up seperately. - Rajat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50600/#review144728 ----------------------------------------------------------- On Aug. 4, 2016, 6:30 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50600/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2016, 6:30 p.m.) > > > Review request for lens. > > > Bugs: LENS-1243 > https://issues.apache.org/jira/browse/LENS-1243 > > > Repository: lens > > > Description > ------- > > > Diffs > ----- > > lens-driver-es/src/main/java/org/apache/lens/driver/es/ESDriver.java > 8a4f4105ad6ffc75445e5e10a51f6fbd4f5f1d3f > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java > 1326611d01e7a9b4f58877435f36b1391ca178b2 > > lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestRemoteHiveDriver.java > 6dff173e26993e40e3cb82622be8173359f92592 > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > bebb9ae9a4d694f224f0ec907df166880e0cf5b8 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java > 12fa6f0328242b1bd06ff0397b2f2a3a9dec010d > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java > e7636d2e631cf86021af398ee31300441a335096 > lens-driver-jdbc/src/test/resources/hive-site.xml > b497ca19b14811ecb4474b7f9bf6dec2bdf0b036 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java > f1d844a1987d7f7990b2e10f03d2e3af8e0294e0 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java > 2374c1e4b2737a0d1ea7123232800413baa9a560 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java > 95ea3608e612aff44ec78eee003c05ccd3068ffb > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/QueryCompletionListener.java > 3713b5156e2cfc20528d9b3b9a6f9b117daeb284 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/QueryDriverStatusUpdateListener.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/StatusUpdateMethod.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 2641b606faa20943a5f3b9de9b729380a35e8c25 > > lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java > 59f85691d08e7deb5e3b5892b3ffc729265874d8 > lens-server/src/main/java/org/apache/lens/server/LensServerConf.java > e977ebd82118307bdbfd20be762a523337da4788 > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java > 47159b568e9eff2f39ede9839f278441f0305d1f > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 84dcecda0970d2756a943522a5876117daa71409 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 9f40a014bebdfc6f0ff7ea3d7a21baacc890018d > lens-server/src/test/resources/hive-site.xml > 94c5012821330e1a78aad2a7fdfca0cc1d0fade2 > > Diff: https://reviews.apache.org/r/50600/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
