You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Raju Bairishetti <ra...@gmail.com> on 2016/07/06 04:20:40 UTC

Re: Review Request 49371: LENS-1168 : Add query launcher pool

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




lens-server-api/src/test/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintTest.java (line 48)
<https://reviews.apache.org/r/49371/#comment206248>

    Can we change this to driver.max.concurrent.launchers?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 705)
<https://reviews.apache.org/r/49371/#comment206293>

    We are already setting this in the queryLauncher thread. Seems redundant.



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 706)
<https://reviews.apache.org/r/49371/#comment206294>

    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.


- Raju Bairishetti


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
> 
>


Re: Review Request 49371: LENS-1168 : Add query launcher pool

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> 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
> 
>