----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47007/#review132366 -----------------------------------------------------------
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java (line 149) <https://reviews.apache.org/r/47007/#comment196548> Please updated lensserver-default.xml also. lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java (line 150) <https://reviews.apache.org/r/47007/#comment196549> should we also mention same session, same name and same query parameters ? lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java (line 153) <https://reviews.apache.org/r/47007/#comment196547> should be change the name to lens.server.duplicate.query.allowed lens.server.optimise.on.duplicate.query lens.server.use.first.query.for.duplicate.submission lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java (line 194) <https://reviews.apache.org/r/47007/#comment196550> should we rename this to queryConfigHash or just configHash ? This is hash of serverconfig + session config + query config lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 1780) <https://reviews.apache.org/r/47007/#comment196551> Should we change the name of the method checkQueryInCache()? Its slightly misleading. Some options : checkForRepeteadQuery(). chcekForDuplicateQuery(). lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 1809) <https://reviews.apache.org/r/47007/#comment196552> Please update the code for TODO. Please close the comments form previous reviews which are already addressed. lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 1810) <https://reviews.apache.org/r/47007/#comment196553> LENS-904 "Session handle not found" may store all the queries for a session. In that case we can just iterate over the session specific queries. Need to confirm this with Rajat. lens-server/src/main/java/org/apache/lens/server/util/UtilityMethods.java (line 212) <https://reviews.apache.org/r/47007/#comment196554> Should we return empty byte[] to prevent NP is code ? lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java (line 756) <https://reviews.apache.org/r/47007/#comment196555> I think we can just test for one media type (XML). This will save some test time. Other test cases are already testing all media types for the APIs used in this test case. @Rajat/Amareshwari ... thoughts ? lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java (line 783) <https://reviews.apache.org/r/47007/#comment196556> This is not a guaranteed operation., depends on the stage in which query is. lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java (line 787) <https://reviews.apache.org/r/47007/#comment196557> can we wait for handle1/2 to finish rather than firing one more query ? lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java (line 801) <https://reviews.apache.org/r/47007/#comment196560> Should we also test how query configuration affects the handling of duplicate query? lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java (line 807) <https://reviews.apache.org/r/47007/#comment196558> +1 for comments lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java (line 847) <https://reviews.apache.org/r/47007/#comment196559> What is the run time for this test case ? Is it possible to optimize to reduce the run time Instead of DeferredPersistentResultFormatter we can also use QueryExecutionServiceImpl#pauseQuerySubmitter. This will make sure query does not finish execution. See if this makes ur life simpler. - Puneet Gupta On May 9, 2016, 5:24 p.m., Lavkesh Lahngir wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47007/ > ----------------------------------------------------------- > > (Updated May 9, 2016, 5:24 p.m.) > > > Review request for lens. > > > Bugs: LENS-1019 > https://issues.apache.org/jira/browse/LENS-1019 > > > Repository: lens > > > Description > ------- > > If query is repeated from user - with same query, same name, same conf on the > same session > and earlier is still queued or running, then return the same handle. This > will only handle the case of aysnc queries. > > > Diffs > ----- > > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > 23537cb > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 94b79d0 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 42bd4ab > lens-server/src/main/java/org/apache/lens/server/util/UtilityMethods.java > a6c7b13 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > df13ba2 > > Diff: https://reviews.apache.org/r/47007/diff/ > > > Testing > ------- > > testExecuteAsyncDuplicate() methods tests the following. > 1. Same query, same user and same session returns the same handle. > 2. After query completion, same query firing will give the different handle. > 3. Differnt session and same query will return different handle. > > > Thanks, > > Lavkesh Lahngir > >
