----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32157/#review77024 -----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/driver/cube/RewriteUtil.java <https://reviews.apache.org/r/32157/#comment124802> Not related to this issue, but gauge creation might fail if there are more than one cube query here. lens-cube/src/main/java/org/apache/lens/driver/cube/RewriteUtil.java <https://reviews.apache.org/r/32157/#comment124791> Seems merge issues. The code needs to be put back lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java <https://reviews.apache.org/r/32157/#comment124795> I see the number changed here. I think for failing driver rewritten query should not be set. Can we assert that? lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java <https://reviews.apache.org/r/32157/#comment124793> Can we assert failure cause contains 'Mock fail' ? lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java <https://reviews.apache.org/r/32157/#comment124798> Should be make default value bigger - something in minutes? I'm fine with making it 5 minutes or so to account for bad machines. lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java <https://reviews.apache.org/r/32157/#comment124803> typo in the comment lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java <https://reviews.apache.org/r/32157/#comment124800> Isnt such method already there? Can you add javadoc from where it is called? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124813> Should the order be changed here? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124808> Is the conf here, server conf? In that case can we read the value only once? Or should we make configurable per query? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124806> Why are removing driver from QueryContext? With this we are losing the state fully. lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124807> Estimate can be called on other contexts as well. We can add getLogHandle in AbstractQueryContext which returns user query itself and it can be overriddedn in QueryContext to return queryhandle, PreparedQueryContext to return prepareHandle lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124814> Can we have a new gauge for this parallel run as a whole? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124809> Is this still one per driver? I think yes. In that case useStackName should be true. Also I think the gauge as ALL_CUBE_REWRITES does not make sense any more, can be named differently lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124811> Please log instead of printing. Whats the behavior when the exception is thrown here? Seems right now it is ignored. lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124812> Same as above - Amareshwari Sriramadasu On March 19, 2015, 6:04 a.m., Jaideep dhok wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32157/ > ----------------------------------------------------------- > > (Updated March 19, 2015, 6:04 a.m.) > > > Review request for lens and Amareshwari Sriramadasu. > > > Bugs: LENS-356 > https://issues.apache.org/jira/browse/LENS-356 > > > Repository: lens > > > Description > ------- > > Changes - > 1. Refactored rewrite and estimate calls to return closures instead of > directly computing result. > 2. QueryExecutionService.rewriteAndSelect will compose a chained runnable for > rewrite and estimate and run them in background thread pool > 3. Changed existing tests to match current implementation - mainly changed > asserts for validation in case of failures. Assertions are still there, just > that their verification is changed. > 4. Verified all impacting unit tests they are passing as of now. > 5. Proceeding with drivers which return an estimate within a timeout. If no > drivers return without an estimate, throwing exception > > > Diffs > ----- > > lens-cube/src/main/java/org/apache/lens/driver/cube/RewriteUtil.java > de79423e701d1fa935d7518fc42d128f261b8b46 > lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java > 00a039774b5f9e66db4828800c8f5af2eb9f17b1 > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > cdc5fd72f63de0f6b4555dd5a08e004013ad7bee > > lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java > 3101ed6410646f558431781d0833ecc92bde01dc > > lens-server-api/src/main/java/org/apache/lens/server/api/query/DriverSelectorQueryContext.java > b53c4b982ac55320c27a00e01bd1c5e4170d1263 > > lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java > 12f6833b70188f72c4c5a9904a6e7d108d7e584c > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 8da3fd55a95ee8421c85b9d6002a099014497028 > lens-server/src/main/resources/lensserver-default.xml > 57fbbc71ede1978d59140f7f3224f90ee8dd2403 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 0f20beed578a4aa40b71928199d873e251b81e4e > lens-server/src/test/resources/lens-site.xml > 2adf7628d765e34a989e1bc66138d81ca16f949b > src/site/apt/admin/config.apt 9e06da368c27bca97f1cf51f484ecdf101d7caa8 > > Diff: https://reviews.apache.org/r/32157/diff/ > > > Testing > ------- > > Fixed existing impacted tests due to refactor. > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.093s] > [INFO] Lens .............................................. SUCCESS [1.742s] > [INFO] Lens API .......................................... SUCCESS [5.839s] > [INFO] Lens API for server and extensions ................ SUCCESS [6.307s] > [INFO] Lens Cube ......................................... SUCCESS [2:05.819s] > [INFO] Lens DB storage ................................... SUCCESS [9.854s] > [INFO] Lens Query Library ................................ SUCCESS [5.304s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:34.289s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [17.355s] > [INFO] Lens Server ....................................... SUCCESS [4:21.122s] > [INFO] Lens client ....................................... SUCCESS [20.654s] > [INFO] Lens CLI .......................................... SUCCESS [1:43.459s] > [INFO] Lens Examples ..................................... SUCCESS [0.899s] > [INFO] Lens Distribution ................................. SUCCESS [9.999s] > [INFO] Lens ML Lib ....................................... SUCCESS [44.152s] > [INFO] Lens Regression ................................... SUCCESS [0.453s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 12:50.381s > [INFO] Finished at: Wed Mar 18 08:41:20 UTC 2015 > [INFO] Final Memory: 109M/1226M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Jaideep dhok > >
