> On Jan. 27, 2016, 5:51 a.m., Amareshwari Sriramadasu wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java, > > line 179 > > <https://reviews.apache.org/r/42067/diff/2/?file=1205548#file1205548line179> > > > > Can you add what is timeout for ? Should it named resultTimeout?
<p>Actullay this is execute timeout. Should I name it that? It is used to calculate the time for which InMemoryResult(in case of prefetch only) should not be purged even if its is fully accessed.</p> > On Jan. 27, 2016, 5:51 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 1514 > > <https://reviews.apache.org/r/42067/diff/2/?file=1205549#file1205549line1514> > > > > We should check for successful() instead of finished() As in failed queries will not go through this flow. Makes Sense. I will upadte > On Jan. 27, 2016, 5:51 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 1651 > > <https://reviews.apache.org/r/42067/diff/2/?file=1205549#file1205549line1651> > > > > Can default timeoutMillis be 0, instead of changing all constructors > > and callers ? QueryContext Constructors are not changed. Only the helper methods in QueryExecutionServiceImpl that create the QueryContext were updated. There was 5 calles for this. 4 within the class and one in a test case form what I remember. I felt it was ok. Please comment if we need to provide some more helper methods without timeout . > On Jan. 27, 2016, 5:51 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 1987 > > <https://reviews.apache.org/r/42067/diff/2/?file=1205549#file1205549line1987> > > > > Can there be a case where the notify is missed on this wait, because > > query already finished? That shouldn't happen for query scueessful cases which we are interested in for streaming. For failure cases, we are catching a SQLException in JDBC driver ( Hive seems fine), and if the exection thrown on executing SQL is within the purview of SQLException, failure cases should also get notified correctly (99% should fall in this bucket in case of failure) . If there is a miss, in case of failure the listener should come out of the wait since it is a timed wait. > On Jan. 27, 2016, 5:51 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, > > line 1998 > > <https://reviews.apache.org/r/42067/diff/2/?file=1205549#file1205549line1998> > > > > If the query failed and purged, what is returned to user from this api? Old flow will be followed in that case. Failed = Query Status Failed will be returned ( result and resultSetMetadata will be null ) Purged = Query Status will be set and Persistent Result will be returned if query persists the result, else null. > On Jan. 27, 2016, 5:51 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/resources/lenssession-default.xml, line 218 > > <https://reviews.apache.org/r/42067/diff/2/?file=1205550#file1205550line218> > > > > Shall we make true by default? That would make true for all > > executeWithTimeout queries. Yupo, this Should be fine. - Puneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42067/#review116541 ----------------------------------------------------------- On Jan. 22, 2016, 10:52 a.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42067/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2016, 10:52 a.m.) > > > Review request for lens. > > > Bugs: lens-901 > https://issues.apache.org/jira/browse/lens-901 > > > Repository: lens > > > Description > ------- > > LENS-901 support streaming results on lens > > > Diffs > ----- > > > lens-api/src/main/java/org/apache/lens/api/query/QueryHandleWithResultSet.java > a5da867 > 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 > 2dff9af > 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 > 494bce5 > > 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 > >
