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

2016-07-24 Thread Puneet Gupta 

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


Ship it!




Ship It!

- Puneet Gupta


On July 22, 2016, 4:05 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 22, 2016, 4:05 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  be0e2c8 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 3ce8fc1 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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 
>   lens-server/src/test/resources/logback.xml a345054 
>   src/site/apt/admin/config.apt 852955b 
> 
> Diff: https://reviews.apache.org/r/49371/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

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

2016-07-21 Thread Amareshwari Sriramadasu 

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

(Updated July 22, 2016, 4:05 a.m.)


Review request for lens.


Changes
---

Test failure fixed


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 (updated)
-

  
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/metrics/MetricsService.java
 7fd2d81 
  
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/QueryExecutionService.java
 d10ad09 
  
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/metrics/MetricsServiceImpl.java
 1e8d540 
  
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 be0e2c8 
  lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
e77c7fa 
  lens-server/src/main/resources/lensserver-default.xml 3ce8fc1 
  lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java d5e975a 
  
lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 
0b38517 
  
lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
 0df436a 
  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 
  lens-server/src/test/resources/logback.xml a345054 
  src/site/apt/admin/config.apt 852955b 

Diff: https://reviews.apache.org/r/49371/diff/


Testing
---


Thanks,

Amareshwari Sriramadasu

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

2016-07-21 Thread Amareshwari Sriramadasu 

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

(Updated July 21, 2016, 11:56 a.m.)


Review request for lens.


Changes
---

Review comments incorporated


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 (updated)
-

  
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/metrics/MetricsService.java
 7fd2d81 
  
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/QueryExecutionService.java
 d10ad09 
  
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/metrics/MetricsServiceImpl.java
 1e8d540 
  
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 be0e2c8 
  lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
e77c7fa 
  lens-server/src/main/resources/lensserver-default.xml 3ce8fc1 
  lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java d5e975a 
  
lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 
0b38517 
  
lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
 0df436a 
  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 852955b 

Diff: https://reviews.apache.org/r/49371/diff/


Testing
---


Thanks,

Amareshwari Sriramadasu

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

2016-07-21 Thread Amareshwari Sriramadasu 


> On July 20, 2016, 8:36 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 767
> > 
> >
> > Cancel code may require some synchronization.

Now that launchQuery is synchronized, this should not be required.


- Amareshwari


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


On July 12, 2016, 5:41 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 12, 2016, 5:41 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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

2016-07-21 Thread Amareshwari Sriramadasu 


> On July 20, 2016, 8:53 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 1339
> > 
> >
> > Do we need to await termination of this pool ?
> > 
> > Cons : a query that takes lot of time to lauch will delay the shutdown.
> > 
> > Pros : on startup we do not need to handle the case where we get a 
> > query in LAUNCHING state.

As we are have await for cancellationPool in prepare stopping, we can add 
launcher pool as well. Adding it here.


- Amareshwari


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


On July 12, 2016, 5:41 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 12, 2016, 5:41 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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

2016-07-21 Thread Amareshwari Sriramadasu 


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  lines 787-793
> > 
> >
> > Do we need to synchronize the entire run method on context ? 
> > I can think of only one reason - cancel and launch happening parallely 
> > before this JIRA were synchronized. Not sure if there can be more reasons 
> > to do this.
> > 
> > If we do syncronize , this code will not be needed.
> 
> Amareshwari Sriramadasu wrote:
> Did not want to synchronize on the entire method, as other operations 
> like getStatus or cancelQuery would block threads for this launching thread.

Doing synchronization on the whole launch - same as the earlier flow. cancel 
and updatestatus will wait if launch takes more time.


- Amareshwari


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


On July 12, 2016, 5:41 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 12, 2016, 5:41 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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

2016-07-20 Thread Amareshwari Sriramadasu 


> On July 20, 2016, 8:36 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 773
> > 
> >
> > We might need to hanlde InterruptedException from Furture.cancel(true)

Future.cancel is not throwing any exception. So, I'm catching all exceptions 
here and checking if query is cancelled() or not.


> On July 20, 2016, 8:36 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 821
> > 
> >
> > Should we move this to QueryLauncher.run() finally block just in case 
> > launchQuery() deosn't get called.

makes sense, will update


- Amareshwari


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


On July 12, 2016, 5:41 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 12, 2016, 5:41 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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

2016-07-20 Thread Puneet Gupta 

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




lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 1314)


Do we need to await termination of this pool ?

Cons : a query that takes lot of time to lauch will delay the shutdown.

Pros : on startup we do not need to handle the case where we get a query in 
LAUNCHING state.


- Puneet Gupta


On July 12, 2016, 5:41 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 12, 2016, 5:41 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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

2016-07-20 Thread Puneet Gupta 

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




lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 752)


Cancel code may require some synchronization.



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 758)


We might need to hanlde InterruptedException from Furture.cancel(true)



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 805)


Should we move this to QueryLauncher.run() finally block just in case 
launchQuery() deosn't get called.



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 1489)


can we make this false ? Some threads will alaways be available to launch 
queries


- Puneet Gupta


On July 12, 2016, 5:41 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 12, 2016, 5:41 a.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/metrics/MetricsService.java
>  7fd2d81 
>   
> 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/QueryExecutionService.java
>  d10ad09 
>   
> 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/metrics/MetricsServiceImpl.java
>  1e8d540 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  2de098d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> e77c7fa 
>   lens-server/src/main/resources/lensserver-default.xml 6dc322e 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java 
> d5e975a 
>   
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java
>  0b38517 
>   
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
>  0df436a 
>   
> 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

2016-07-11 Thread Amareshwari Sriramadasu 

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

(Updated July 12, 2016, 5:41 a.m.)


Review request for lens.


Changes
---

Incorporated review comments


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 (updated)
-

  
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/metrics/MetricsService.java
 7fd2d81 
  
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/QueryExecutionService.java
 d10ad09 
  
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/metrics/MetricsServiceImpl.java
 1e8d540 
  
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 2de098d 
  lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
e77c7fa 
  lens-server/src/main/resources/lensserver-default.xml 6dc322e 
  lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java d5e975a 
  
lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 
0b38517 
  
lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java
 0df436a 
  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

2016-07-11 Thread Amareshwari Sriramadasu 


> On July 11, 2016, 6:12 a.m., Rajat Khandelwal wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java,
> >  line 205
> > 
> >
> > I feel adding a new state would be better than adding a boolean which 
> > is true only for an intermediate phase between two particular states, and 
> > false every other time. Having the boolean makes understanding the stata 
> > machine difficult for new people. 
> > 
> > Then state machine would be 
> > `NEW->QUEUED->LAUNCHING->LAUNCHED->RUNNING->...`

There are both pros and cons. Adding a new state can make the code obvious, but 
will touch all pieces of code.
Doing a state change would make it incompatible and end users have to 
understand new state more. And the use of LAUNCHING state is useful only for 
constraint checks, as of now.


- Amareshwari


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


On July 6, 2016, 8:47 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 6, 2016, 8:47 a.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

2016-07-11 Thread Rajat Khandelwal 

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




lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java
 (line 205)


I feel adding a new state would be better than adding a boolean which is 
true only for an intermediate phase between two particular states, and false 
every other time. Having the boolean makes understanding the stata machine 
difficult for new people. 

Then state machine would be `NEW->QUEUED->LAUNCHING->LAUNCHED->RUNNING->...`



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 1461)


We'll need to stop the pool in `prepareStopping`


- Rajat Khandelwal


On July 6, 2016, 2:17 p.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 6, 2016, 2:17 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

2016-07-11 Thread Amareshwari Sriramadasu 


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java,
> >  line 49
> > 
> >
> > shoudl we use driverLaunchedQueries.size() here.

Not changing as there are tests mocking it.


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintFactory.java,
> >  line 76
> > 
> >
> > Should we have a default value of 10/20 ?

Instead of 10/20, planning to default to max concurrent queries.


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 2538
> > 
> >
> > should we log the query id or will it come from MDC ?

Yes. Would come from MDC. Updated in all callers


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  lines 787-793
> > 
> >
> > Do we need to synchronize the entire run method on context ? 
> > I can think of only one reason - cancel and launch happening parallely 
> > before this JIRA were synchronized. Not sure if there can be more reasons 
> > to do this.
> > 
> > If we do syncronize , this code will not be needed.

Did not want to synchronize on the entire method, as other operations like 
getStatus or cancelQuery would block threads for this launching thread.


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 1484
> > 
> >
> > Do we need to have metrics around laucher pool(number of active thread, 
> > queries in launching state) ?

Not planning to add any such at the pool level. Thread dump would give details 
of all those. We can add gauge for queries being launched. Will try adding one.


> On July 8, 2016, 10:45 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  line 1493
> > 
> >
> > do we need to make this class level varaible ?

No. The number here is mainly for thread number in launcher pool alone. Not 
across all pools.


- Amareshwari


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


On July 6, 2016, 8:47 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 6, 2016, 8:47 a.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 
>

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

2016-07-08 Thread Puneet Gupta 

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




lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java
 (line 49)


shoudl we use driverLaunchedQueries.size() here.



lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java
 (line 97)


Suggestion : should we change the name to getIsLauchingCount()



lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintFactory.java
 (line 76)


Should we have a default value of 10/20 ?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 2535)


should we log the query id or will it come from MDC ?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 659)


Should we remove errorCollection from constructor too ?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 752)


org.apache.lens.server.session.LensSessionImpl#acquire is synchronized . 
Should we look at reducing the scope of synchronization here or in another JIRA 
?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (lines 773 - 779)


Do we need to synchronize the entire run method on context ? 
I can think of only one reason - cancel and launch happening parallely 
before this JIRA were synchronized. Not sure if there can be more reasons to do 
this.

If we do syncronize , this code will not be needed.



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 801)


should we do this in finally{}



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 1461)


Do we need to have metrics around laucher pool(number of active thread, 
queries in launching state) ?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 1470)


do we need to make this class level varaible ?


- Puneet Gupta


On July 6, 2016, 8:47 a.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49371/
> ---
> 
> (Updated July 6, 2016, 8:47 a.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

2016-07-06 Thread Amareshwari Sriramadasu 

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

(Updated July 6, 2016, 8:47 a.m.)


Review request for lens.


Changes
---

Incorporated review comments.


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 (updated)
-

  
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

2016-07-06 Thread Amareshwari Sriramadasu 


> On July 6, 2016, 5:29 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java,
> >  line 939
> > 
> >
> > Should we increase number this since we are using a SynchronousQueue 
> > for laucherPool

Increasing to 100.


> On July 6, 2016, 5:29 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java,
> >  line 40
> > 
> >
> > should we call this maxConcurrentLaunches  and 
> > driver.max.concurrent.launches

Updated


> On July 6, 2016, 5:29 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraint.java,
> >  line 47
> > 
> >
> > Should we cache launchedQueries.getQueries(selectedDriver) locally and 
> > use it for condition 1 and 2

Updated


> On July 6, 2016, 5:29 a.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/constraint/MaxConcurrentDriverQueriesConstraintFactory.java,
> >  lines 36-42
> > 
> >
> > 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.

Not planning to do this.


> On July 6, 2016, 5:29 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java,
> >  lines 661-665
> > 
> >
> > errorCOllection is not required ?

yes. Not required.


- Amareshwari


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


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

2016-07-06 Thread Amareshwari Sriramadasu 


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

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

2016-07-05 Thread Puneet Gupta 

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


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)


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)


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)


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)


Duplicate



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (lines 659 - 662)


errorCOllection is not required ?



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 707)


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)


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)


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

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

2016-07-05 Thread Raju Bairishetti 

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


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



lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
 (line 705)


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)


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

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

2016-06-29 Thread Amareshwari Sriramadasu 

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

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