> On July 6, 2016, 4:20 a.m., Raju Bairishetti wrote:
> > lens-server-api/src/test/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintTest.java,
> >  line 48
> > <https://reviews.apache.org/r/49371/diff/1/?file=1432920#file1432920line48>
> >
> >     Can we change this to driver.max.concurrent.launchers?

Thanks for catching. Updating.


> On July 6, 2016, 4:20 a.m., Raju Bairishetti wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 712
> > <https://reviews.apache.org/r/49371/diff/1/?file=1432921#file1432921line712>
> >
> >     We are already setting this in the queryLauncher thread. Seems 
> > redundant.

Removing it from queryLauncher thread. It will be required here, as launching 
constraint checker would check for this flag to know if it is launching.


> On July 6, 2016, 4:20 a.m., Raju Bairishetti wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 713
> > <https://reviews.apache.org/r/49371/diff/1/?file=1432921#file1432921line713>
> >
> >     I feel this should be done by launcher thread only.
> >        I do not have much context about query constraints stuff. Just 
> > posting some thoughts here to understand little bit more about query 
> > constraints.
> >        
> >         Seems like adding here may violates the launcher queries 
> > constraints. Some times queries won't be launched even though none of the 
> > queries are in laucnhed state because of launchedQueries contains the 
> > queries which did not launch yet.
> >     
> >       *My Assumptions*: If all threads are busy in executing the queries 
> > and meanwhile mutliple queries are submitted then it launcher pool may not 
> > launch any of the queries after finishing the existing queries as it will 
> > not meet max launched queries constraint.

This has to be done before launching, for the constraint checker to check how 
many parallel launches are happening. This is to have limit on the parallel 
launches.

>> Seems like adding here may violates the launcher queries constraints. Some 
>> times queries won't be launched even though none of the queries are in 
>> laucnhed state because of launchedQueries contains the queries which did not 
>> launch yet.

No. QueryLauncher is not checking for any constraints, QuerySubmitter does. 
Once QuerySubmitter decides it can be launched, we can add them for parallel 
launch constraint to work.

>> My Assumptions: If all threads are busy in executing the queries and 
>> meanwhile mutliple queries are submitted then it launcher pool may not 
>> launch any of the queries after finishing the existing queries as it will 
>> not meet max launched queries constraint.

All threads will only do launch (asynchronously) and come out. They are not 
living around while executing.


- Amareshwari


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49371/#review140896
-----------------------------------------------------------


On June 29, 2016, 12:15 p.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> -----------------------------------------------------------
> 
> (Updated June 29, 2016, 12:15 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1168
>     https://issues.apache.org/jira/browse/LENS-1168
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Changes include :
> - Add query launcher pool which would launch queries after applying 
> constraint checks.
> - Add max concurrent launching queries constraint - to limit number parallel 
> launches at any time.
> - Update StatusPoller to skip launching queries
> - Update cancelQuery to interrupt launcher.
> - Added test to verify parallel launches.
> 
> 
> Diffs
> -----
> 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
>  bd9b1ab 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java
>  8ba0689 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java
>  b2319a9 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintFactory.java
>  442cd99 
>   
> lens-server-api/src/test/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintTest.java
>  38b74ae 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 
> 6af4225 
>   
> lens-server/src/test/java/org/apache/lens/server/query/constraint/ThreadSafeEstimatedQueryCollectionTest.java
>  c8ebd0c 
>   src/site/apt/admin/config.apt 5b76069 
> 
> Diff: https://reviews.apache.org/r/49371/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

Reply via email to