You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Lavkesh Lahngir <la...@gmail.com> on 2017/04/21 07:25:04 UTC
Review Request 58604: Handle Estimates failing with task rejection
errors
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-1413
https://issues.apache.org/jira/browse/LENS-1413
Repository: lens
Description
-------
1. Cancel estimates for other drivers when the estimate pool queue is full.
2. Return the appropriate error message
Diffs
-----
lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java 92724c6
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
lens-server/src/test/resources/lens-site.xml 334a9c4
Diff: https://reviews.apache.org/r/58604/diff/1/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On April 21, 2017, 9:34 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
> > Lines 1716 (patched)
> > <https://reviews.apache.org/r/58604/diff/1/?file=1697059#file1697059line1716>
> >
> > Not sure if logging the driver name makes sense. Also Should we log this as error or warning is debatable ?
hmm. I guess logging does not hurt.. we can see which ones were rejected and which ones were cancelled.
> On April 21, 2017, 9:34 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
> > Lines 1722 (patched)
> > <https://reviews.apache.org/r/58604/diff/1/?file=1697059#file1697059line1722>
> >
> > can we use for(RewriteEstimateRunnable r : runnables) instead ?
uh. In this loop we are fetching from two lists.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/#review172619
-----------------------------------------------------------
On April 21, 2017, 12:10 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58604/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 12:10 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1413
> https://issues.apache.org/jira/browse/LENS-1413
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Cancel estimates for other drivers when the estimate pool queue is full.
> 2. Return the appropriate error message
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/lens-errors.conf 236d678
> lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java 14a31e2
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
> lens-server/src/test/resources/lens-site.xml 334a9c4
>
>
> Diff: https://reviews.apache.org/r/58604/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/#review172619
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
Lines 1716 (patched)
<https://reviews.apache.org/r/58604/#comment245704>
Not sure if logging the driver name makes sense. Also Should we log this as error or warning is debatable ?
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
Lines 1722 (patched)
<https://reviews.apache.org/r/58604/#comment245702>
can we use for(RewriteEstimateRunnable r : runnables) instead ?
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
Lines 1727 (patched)
<https://reviews.apache.org/r/58604/#comment245703>
Alsoe we can add a sugestion to "Please try again later".
- Puneet Gupta
On April 21, 2017, 7:25 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58604/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 7:25 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1413
> https://issues.apache.org/jira/browse/LENS-1413
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Cancel estimates for other drivers when the estimate pool queue is full.
> 2. Return the appropriate error message
>
>
> Diffs
> -----
>
> lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java 92724c6
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
> lens-server/src/test/resources/lens-site.xml 334a9c4
>
>
> Diff: https://reviews.apache.org/r/58604/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On April 21, 2017, 11:49 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java
> > Lines 68 (patched)
> > <https://reviews.apache.org/r/58604/diff/2/?file=1697178#file1697178line68>
> >
> > Should not be going into Cube errors, should be moved to LensServerErrorCode.
> >
> > And accordingly change error code and position in lens-errors
sure.
> On April 21, 2017, 11:49 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
> > Lines 1988 (patched)
> > <https://reviews.apache.org/r/58604/diff/2/?file=1697181#file1697181line2100>
> >
> > why is it only 10, when 20 threads are launched?
Typo.
> On April 21, 2017, 11:49 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
> > Lines 1989 (patched)
> > <https://reviews.apache.org/r/58604/diff/2/?file=1697181#file1697181line2101>
> >
> > Should we be joining for all vs interrupt them and join? Interrupting would reduce test time.
Only few threads will go to FailQueryDriver. Interrupting might not simulate the rejection error.
> On April 21, 2017, 11:49 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
> > Lines 1991 (patched)
> > <https://reviews.apache.org/r/58604/diff/2/?file=1697181#file1697181line2103>
> >
> > Add assert for howmany failed as well.
number of failures are not totally deterministic.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/#review172628
-----------------------------------------------------------
On April 21, 2017, 11:40 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58604/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 11:40 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1413
> https://issues.apache.org/jira/browse/LENS-1413
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Cancel estimates for other drivers when the estimate pool queue is full.
> 2. Return the appropriate error message
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/lens-errors.conf 236d678
> lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java babe3de
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
> lens-server/src/test/resources/lens-site.xml 334a9c4
>
>
> Diff: https://reviews.apache.org/r/58604/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/#review172628
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java
Lines 68 (patched)
<https://reviews.apache.org/r/58604/#comment245707>
Should not be going into Cube errors, should be moved to LensServerErrorCode.
And accordingly change error code and position in lens-errors
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
Lines 1988 (patched)
<https://reviews.apache.org/r/58604/#comment245709>
why is it only 10, when 20 threads are launched?
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
Lines 1989 (patched)
<https://reviews.apache.org/r/58604/#comment245710>
Should we be joining for all vs interrupt them and join? Interrupting would reduce test time.
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
Lines 1991 (patched)
<https://reviews.apache.org/r/58604/#comment245708>
Add assert for howmany failed as well.
- Amareshwari Sriramadasu
On April 21, 2017, 11:40 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58604/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 11:40 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1413
> https://issues.apache.org/jira/browse/LENS-1413
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Cancel estimates for other drivers when the estimate pool queue is full.
> 2. Return the appropriate error message
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/lens-errors.conf 236d678
> lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java babe3de
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
> lens-server/src/test/resources/lens-site.xml 334a9c4
>
>
> Diff: https://reviews.apache.org/r/58604/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/#review172774
-----------------------------------------------------------
Ship it!
Ship It!
- Amareshwari Sriramadasu
On April 24, 2017, 11:43 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58604/
> -----------------------------------------------------------
>
> (Updated April 24, 2017, 11:43 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1413
> https://issues.apache.org/jira/browse/LENS-1413
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Cancel estimates for other drivers when the estimate pool queue is full.
> 2. Return the appropriate error message
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/lens-errors.conf 236d678
> lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java 14a31e2
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
> lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
> lens-server/src/test/resources/lens-site.xml 334a9c4
>
>
> Diff: https://reviews.apache.org/r/58604/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/
-----------------------------------------------------------
(Updated April 24, 2017, 11:43 a.m.)
Review request for lens.
Bugs: LENS-1413
https://issues.apache.org/jira/browse/LENS-1413
Repository: lens
Description
-------
1. Cancel estimates for other drivers when the estimate pool queue is full.
2. Return the appropriate error message
Diffs (updated)
-----
lens-api/src/main/resources/lens-errors.conf 236d678
lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java 14a31e2
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
lens-server/src/test/resources/lens-site.xml 334a9c4
Diff: https://reviews.apache.org/r/58604/diff/4/
Changes: https://reviews.apache.org/r/58604/diff/3-4/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/
-----------------------------------------------------------
(Updated April 21, 2017, 12:10 p.m.)
Review request for lens.
Bugs: LENS-1413
https://issues.apache.org/jira/browse/LENS-1413
Repository: lens
Description
-------
1. Cancel estimates for other drivers when the estimate pool queue is full.
2. Return the appropriate error message
Diffs (updated)
-----
lens-api/src/main/resources/lens-errors.conf 236d678
lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java 14a31e2
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
lens-server/src/test/resources/lens-site.xml 334a9c4
Diff: https://reviews.apache.org/r/58604/diff/3/
Changes: https://reviews.apache.org/r/58604/diff/2-3/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/
-----------------------------------------------------------
(Updated April 21, 2017, 11:40 a.m.)
Review request for lens.
Bugs: LENS-1413
https://issues.apache.org/jira/browse/LENS-1413
Repository: lens
Description
-------
1. Cancel estimates for other drivers when the estimate pool queue is full.
2. Return the appropriate error message
Diffs (updated)
-----
lens-api/src/main/resources/lens-errors.conf 236d678
lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java babe3de
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
lens-server/src/test/java/org/apache/lens/server/common/FailingQueryDriver.java 7e9133e
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
lens-server/src/test/resources/lens-site.xml 334a9c4
Diff: https://reviews.apache.org/r/58604/diff/2/
Changes: https://reviews.apache.org/r/58604/diff/1-2/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 58604: Handle Estimates failing with task
rejection errors
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58604/#review172607
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
Lines 1727 (patched)
<https://reviews.apache.org/r/58604/#comment245692>
Should have a proper error code associated and message should go to lens-errors.conf
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
Lines 2044-2046 (patched)
<https://reviews.apache.org/r/58604/#comment245695>
We can use FailingQueryDriver instead of changing server state like this.
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
Lines 2051-2054 (patched)
<https://reviews.apache.org/r/58604/#comment245696>
Can we call estimate api instead of this and validate the error response from api?
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java
Lines 2065 (patched)
<https://reviews.apache.org/r/58604/#comment245697>
Calling sleeping estimates in for loop will do than running them as Runnables.
- Amareshwari Sriramadasu
On April 21, 2017, 7:25 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58604/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 7:25 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1413
> https://issues.apache.org/jira/browse/LENS-1413
>
>
> Repository: lens
>
>
> Description
> -------
>
> 1. Cancel estimates for other drivers when the estimate pool queue is full.
> 2. Return the appropriate error message
>
>
> Diffs
> -----
>
> lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java 92724c6
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java c6fbeda
> lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java 7d8c977
> lens-server/src/test/resources/lens-site.xml 334a9c4
>
>
> Diff: https://reviews.apache.org/r/58604/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>