> On Feb. 2, 2016, 10:09 a.m., Rajat Khandelwal wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 549 > > <https://reviews.apache.org/r/42067/diff/3/?file=1227569#file1227569line549> > > > > I think `!driverRS.equals(serverRS)` should be used instead of `!=`.
Since there is only one result set instance now which is returned by driver on each fetch call ( as it is chached in query context), the check is intentional. > On Feb. 2, 2016, 10:09 a.m., Rajat Khandelwal wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java, > > line 191 > > <https://reviews.apache.org/r/42067/diff/3/?file=1227568#file1227568line191> > > > > It's transient, what happens to this field on restart? If it's getting > > used in critical paths, we'll need to make sure it gets restored properly. It will be null on restart. That is Ok as we don't want to serialize the result set. It can be re set if driver (hive) finishes executing query after server restarts. > On Feb. 2, 2016, 10:09 a.m., Rajat Khandelwal wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java, > > line 82 > > <https://reviews.apache.org/r/42067/diff/3/?file=1227565#file1227565line82> > > > > How do we enforce that a driver implements at least one of `fetch` or > > `create`? Cerate in abstract driver will throw exception now forcing the implementer to override it. Duplicate of amareshwari's comment ( create should not return null ) > On Feb. 2, 2016, 10:09 a.m., Rajat Khandelwal wrote: > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java, > > line 425 > > <https://reviews.apache.org/r/42067/diff/3/?file=1227563#file1227563line425> > > > > Why the extra row? Since we do not stream partial results, there is no other way to know if result is pre fetched partailly. - Puneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42067/#review117386 ----------------------------------------------------------- On Feb. 1, 2016, 7:19 a.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42067/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2016, 7:19 a.m.) > > > Review request for lens. > > > Bugs: lens-901 > https://issues.apache.org/jira/browse/lens-901 > > > Repository: lens > > > Description > ------- > > support streaming results on lens > > > Diffs > ----- > > > lens-api/src/main/java/org/apache/lens/api/query/QueryHandleWithResultSet.java > a5da867 > lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 44fd97e > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java > 149c6ab > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > 82d7513 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java > b96cf88 > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > a3dbfc0 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java > ed1fc43 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/InMemoryResultSet.java > c64a3dd > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/PartiallyFetchedInMemoryResultSet.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 1269e45 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 672f2be > lens-server/src/main/resources/lenssession-default.xml a321c3f > lens-server/src/test/java/org/apache/lens/server/query/TestLensDAO.java > 01e846a > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 3facded > > lens-server/src/test/java/org/apache/lens/server/query/TestResultFormatting.java > 30d1e34 > src/site/apt/admin/config.apt 54f827e > src/site/apt/admin/session-config.apt 05a2c2c > > Diff: https://reviews.apache.org/r/42067/diff/ > > > Testing > ------- > > Three properties have been added to enable streaming > > * lens.query.prefetch.inmemory.resultset > When set to true, specified number of rows of in-memory result set will be > pre-fetched > > * lens.query.prefetch.inmemory.resultset.rows > Specifies the number of rows to pre-fetch for in-memory result set when > lens.query.prefetch.inmemory.resultset is set to true. Default value is 100 > rows. > > * lens.query.prefetch.inmemory.resultset.ttl.millis Specifies the time in > milli seconds starting from query submission time for which the pre-fetched > in-memory result set will not be purged. This property is put to action only > when lens.query.prefetch.inmemory.resultset is set to true and all rows of > the in memory result set have been pre-fetched. The default value of this > property is 60 secs. > > Streaming(pre-fetched) result set is available in case the above proerties > are set and the result is a type of InMemoryResultSet. Further, the streming > is enabled only when all result rows have been prefetched.Partial Streaming > not supported as of now (Can be enabled in furtue is there is a clear use > case for it) > > Also note QueryHandleWithResultSet will now hold result metadta > (QueryResultSetMetadata) also. > This info will be useful for client in case the streaming results are > accesesd via execute_with_timeout API (REST End Point /queryapi/queries POST ) > > It is suggested to use this feature for queries that finish fast and return > small number of rows. Streaming and persistence by server can happen > parallely for streamed result sets. > > Will add more details on **Testing** done . Was able to build and run > checkstyle. Also ran the added test cases for this feature. > > **TODO** PartiallyFetchedInMemoryResultSet can override > InMemoryResultSet.toQueryResult() and optimize the case where result has been > completely fetched already. > > > Thanks, > > Puneet Gupta > >
