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




lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
(line 939)
<https://reviews.apache.org/r/49371/#comment206328>

    Should we increase number this since we are using a SynchronousQueue for 
laucherPool



lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java
 (line 40)
<https://reviews.apache.org/r/49371/#comment206325>

    should we call this maxConcurrentLaunches  and 
driver.max.concurrent.launches



lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java
 (line 47)
<https://reviews.apache.org/r/49371/#comment206301>

    Should we cache launchedQueries.getQueries(selectedDriver) locally and use 
it for condition 1 and 2



lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintFactory.java
 (lines 35 - 41)
<https://reviews.apache.org/r/49371/#comment206327>

    driver.max.concurrent.launched.queries and 
    driver.max.concurrent.launches  seem similar. 
    
    Not sure how we should name them. Should we rename 
"driver.max.concurrent.launched.queries" to 
"driver.max.concurrent.running.queries" but this will be an incompatible change.



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

    Duplicate



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (lines 659 - 662)
<https://reviews.apache.org/r/49371/#comment206309>

    errorCOllection is not required ?



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

    Should we store the Future object in QuereyContext instead and use it to 
cancel the task via Future.cancel(true)? Future#isCancelled can be logged after 
this call



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (lines 759 - 766)
<https://reviews.apache.org/r/49371/#comment206312>

    Do we need to hanlde InterrupedException separately and check if that was 
due to a cancel on this query ?



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

    should we disable this and let the core pool be available always for faster 
launching.


- Puneet Gupta


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