----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32157/#review76875 -----------------------------------------------------------
lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java <https://reviews.apache.org/r/32157/#comment124575> Is this needed? Can this always be figured out using `queryContext` and `driver`? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java <https://reviews.apache.org/r/32157/#comment124576> Can we abstract some things out to create just a ChainRunnable? Then implement `RewriteEstimateRunnable` using that? Few things: 1. Can we take out common code from the three runnable implementations and make a base class, say `DriverExecutable` ? Generics might be of help. 2. Make an abstract class `DriverExecuter` for executing a collection of `DriverExecutable`s 3. Two implementations of `DriverExecuter`: sequential and parallel. Each have some configurations. e.g. parallel implementation can take some config that specifies whether to run all or run as much as possible in given time frame. 4. Ensure separation of concern between the `DriverExecuter` and its callers. - Rajat Khandelwal On March 17, 2015, 5:03 p.m., Jaideep dhok wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32157/ > ----------------------------------------------------------- > > (Updated March 17, 2015, 5:03 p.m.) > > > Review request for lens and Amareshwari Sriramadasu. > > > Bugs: LENS-356 > https://issues.apache.org/jira/browse/LENS-356 > > > Repository: lens > > > Description > ------- > > This is WIP. Raising review req to get early comments. > > 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. > > Known pending items - > 1. Config entry for parallel call timeout and its documentation > 2. Decide right default value for timeout call. > 2. In case estimate call times out for one driver, but the other driver is > complete, should we return driver which has completed? Or indicate to user > that estimate has failed? right now I am throwing error in estimate call. > > > 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/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 > e3bfed817ed988d7e73e802199a1d649f38418be > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 0f20beed578a4aa40b71928199d873e251b81e4e > lens-server/src/test/resources/lens-site.xml > 2adf7628d765e34a989e1bc66138d81ca16f949b > > Diff: https://reviews.apache.org/r/32157/diff/ > > > Testing > ------- > > Fixed existing impacted tests due to refactor. > > > Thanks, > > Jaideep dhok > >
