You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2015/12/21 10:15:24 UTC
Review Request 41506: LENS-899: Create Attempt framework
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-899
https://issues.apache.org/jira/browse/LENS-899
Repository: lens
Description
-------
Diffs
-----
lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
Diff: https://reviews.apache.org/r/41506/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Dec. 30, 2015, 2:31 p.m., Puneet Gupta wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java, line 34
> > <https://reviews.apache.org/r/41506/diff/1-2/?file=1173731#file1173731line34>
> >
> > can this use attempt number for comparison ? Also who is using this comparator now ?
1. This comparator is not being used.
2. I don't want to store its index in the list inside the object.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review112292
-----------------------------------------------------------
On Dec. 24, 2015, 4:06 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2015, 4:06 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review112292
-----------------------------------------------------------
lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java (line 34)
<https://reviews.apache.org/r/41506/#comment172712>
can this use attempt number for comparison ? Also who is using this comparator now ?
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 837)
<https://reviews.apache.org/r/41506/#comment172713>
Server restart cases may need some wiring if query has failed and the could not be relaunched as server crashed/restarted. On server startup, we will have to identify such cases and handle appropriately.
- Puneet Gupta
On Dec. 24, 2015, 10:36 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2015, 10:36 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/
-----------------------------------------------------------
(Updated Dec. 24, 2015, 4:06 p.m.)
Review request for lens.
Bugs: LENS-899
https://issues.apache.org/jira/browse/LENS-899
Repository: lens
Description
-------
Diffs (updated)
-----
lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
Diff: https://reviews.apache.org/r/41506/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 834
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line834>
> >
> > Shall we let the driver decide on number of re-tries ? It can be diff for Hive and JDBC. Eventually (in future) it can even differ based on error type.
Sure, but I believe the server should impose some upper bound on retries. That's why I've kept default retries to 3. That's server's bound, the drivers have full freedom apart from that.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review111459
-----------------------------------------------------------
On Dec. 21, 2015, 2:45 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2015, 2:45 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Puneet Gupta <pu...@gmail.com>.
> On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, line 162
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173734#file1173734line162>
> >
> > This can be a static string. We can create it once and reuse. Same can be follwed for other instances too.
Dropping . This string is already Internalized.
> On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, line 207
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173734#file1173734line207>
> >
> > driverendtime can be skipped from ordering construct.
>
> Rajat Khandelwal wrote:
> Added it for a currently impossible (but plausible) bug, when driver start time somehow gets missed while updating in the table, but driver end time is updated correctly. Keeping both in order by will allow the attempts to be ordered. Let me know if you still feel it should be removed.
Should we have a separate column then , something like attempt_num.
It ll be more clear when we look at the DB Table.
> On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 834
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line834>
> >
> > Shall we let the driver decide on number of re-tries ? It can be diff for Hive and JDBC. Eventually (in future) it can even differ based on error type.
>
> Rajat Khandelwal wrote:
> Sure, but I believe the server should impose some upper bound on retries. That's why I've kept default retries to 3. That's server's bound, the drivers have full freedom apart from that.
Not sure if server should restrict the driver. A driver may decide for a particualr query/error it can try more than 3 times. The driver's shouldRetry() method can decide in that case ?
> On Dec. 22, 2015, 1:55 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 1857
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line1857>
> >
> > Should we store numOfAttempts in finishedQueries table? If the value is zero, we need not check failed attempts table at all. Also admin can query finished query table and gets attempts statistics easily.
> >
> > Or is there a way to add a new Query in lensServerDao which joins finished queries table with failed attempts and gets the result in one hop. It can be messy though.
>
> Rajat Khandelwal wrote:
> I don't think it'll be messy. `select * from finished_queries where handle='handle'` will now be written as `select *, (count(*) from failed_attempts where handle='handle') failed_attempts from finished_queries where handle='handle'`
>
> Though it *will* be messy without the handle filter which it will still be even after addition of the `num_attempts` field.
I meant that can we can query the DB just once for 99% cases where attempts are 0 ? As of now we have to query twice in all cases. Once finished queries table and once fialed attempts table.
- Puneet
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review111459
-----------------------------------------------------------
On Dec. 21, 2015, 9:15 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2015, 9:15 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, line 207
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173734#file1173734line207>
> >
> > driverendtime can be skipped from ordering construct.
Added it for a currently impossible (but plausible) bug, when driver start time somehow gets missed while updating in the table, but driver end time is updated correctly. Keeping both in order by will allow the attempts to be ordered. Let me know if you still feel it should be removed.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 820
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line820>
> >
> > Will the relaunch we immediate or it ll have some time gap?
I've implemented only immediate re-launch right now. This allows me to keep the querycontext in `launchedQueries` and not maintain a new data structure. For delayed launch, It'll make sense to allow other queries before the actual re-launch. That'll require me to remove the current query from `launchedQueries` and let the server progress. Then there will be another data structure and (possibly) another thread to look through that. Seeing the complexeity, I've gone with the immediate re-launch.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 837
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line837>
> >
> > Can we add an info log that query is being relaunched.
Added logs in re-launch success and failed scenarios.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 1857
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line1857>
> >
> > Should we store numOfAttempts in finishedQueries table? If the value is zero, we need not check failed attempts table at all. Also admin can query finished query table and gets attempts statistics easily.
> >
> > Or is there a way to add a new Query in lensServerDao which joins finished queries table with failed attempts and gets the result in one hop. It can be messy though.
I don't think it'll be messy. `select * from finished_queries where handle='handle'` will now be written as `select *, (count(*) from failed_attempts where handle='handle') failed_attempts from finished_queries where handle='handle'`
Though it *will* be messy without the handle filter which it will still be even after addition of the `num_attempts` field.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review111459
-----------------------------------------------------------
On Dec. 21, 2015, 2:45 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2015, 2:45 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 834
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line834>
> >
> > Shall we let the driver decide on number of re-tries ? It can be diff for Hive and JDBC. Eventually (in future) it can even differ based on error type.
>
> Rajat Khandelwal wrote:
> Sure, but I believe the server should impose some upper bound on retries. That's why I've kept default retries to 3. That's server's bound, the drivers have full freedom apart from that.
>
> Puneet Gupta wrote:
> Not sure if server should restrict the driver. A driver may decide for a particualr query/error it can try more than 3 times. The driver's shouldRetry() method can decide in that case ?
I feel there's no harm in adding an option to specify upper limit. In a deployment that upper limit can easily be `Long.MAX_VALUE`. Or I can change that to be the default value.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java, line 1857
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173735#file1173735line1857>
> >
> > Should we store numOfAttempts in finishedQueries table? If the value is zero, we need not check failed attempts table at all. Also admin can query finished query table and gets attempts statistics easily.
> >
> > Or is there a way to add a new Query in lensServerDao which joins finished queries table with failed attempts and gets the result in one hop. It can be messy though.
>
> Rajat Khandelwal wrote:
> I don't think it'll be messy. `select * from finished_queries where handle='handle'` will now be written as `select *, (count(*) from failed_attempts where handle='handle') failed_attempts from finished_queries where handle='handle'`
>
> Though it *will* be messy without the handle filter which it will still be even after addition of the `num_attempts` field.
>
> Puneet Gupta wrote:
> I meant that can we can query the DB just once for 99% cases where attempts are 0 ? As of now we have to query twice in all cases. Once finished queries table and once fialed attempts table.
Added that check.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review111459
-----------------------------------------------------------
On Dec. 21, 2015, 2:45 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2015, 2:45 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Dec. 22, 2015, 7:25 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java, line 207
> > <https://reviews.apache.org/r/41506/diff/1/?file=1173734#file1173734line207>
> >
> > driverendtime can be skipped from ordering construct.
>
> Rajat Khandelwal wrote:
> Added it for a currently impossible (but plausible) bug, when driver start time somehow gets missed while updating in the table, but driver end time is updated correctly. Keeping both in order by will allow the attempts to be ordered. Let me know if you still feel it should be removed.
>
> Puneet Gupta wrote:
> Should we have a separate column then , something like attempt_num.
> It ll be more clear when we look at the DB Table.
Sure, will add.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review111459
-----------------------------------------------------------
On Dec. 21, 2015, 2:45 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2015, 2:45 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 41506: LENS-899: Create Attempt framework
Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41506/#review111459
-----------------------------------------------------------
lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java (line 882)
<https://reviews.apache.org/r/41506/#comment171682>
Should we have just one retry by default ?
lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java (line 111)
<https://reviews.apache.org/r/41506/#comment171664>
can we log this as error ?
lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java (line 162)
<https://reviews.apache.org/r/41506/#comment171665>
This can be a static string. We can create it once and reuse. Same can be follwed for other instances too.
lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java (line 207)
<https://reviews.apache.org/r/41506/#comment171666>
driverendtime can be skipped from ordering construct.
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 816)
<https://reviews.apache.org/r/41506/#comment171679>
Will the relaunch we immediate or it ll have some time gap?
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 817)
<https://reviews.apache.org/r/41506/#comment171675>
Can we also log Relauch Failed in else part or in reLaunchFailedQuery().
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 830)
<https://reviews.apache.org/r/41506/#comment171767>
Shall we let the driver decide on number of re-tries ? It can be diff for Hive and JDBC. Eventually (in future) it can even differ based on error type.
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 833)
<https://reviews.apache.org/r/41506/#comment171673>
Can we add an info log that query is being relaunched.
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 1157)
<https://reviews.apache.org/r/41506/#comment171676>
Log msg needs to be updated. May be separate try catch for each table creation .
Also log level should be upgraded to error
lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 1853)
<https://reviews.apache.org/r/41506/#comment171678>
Should we store numOfAttempts in finishedQueries table? If the value is zero, we need not check failed attempts table at all. Also admin can query finished query table and gets attempts statistics easily.
Or is there a way to add a new Query in lensServerDao which joins finished queries table with failed attempts and gets the result in one hop. It can be messy though.
- Puneet Gupta
On Dec. 21, 2015, 9:15 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41506/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2015, 9:15 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-899
> https://issues.apache.org/jira/browse/LENS-899
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 91cbe391a2bc81edae587b268104940122993243
> lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01e3f08b12ff9e872d7412496dfc70ea626
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java ed1fc43ab6e6ef8c34c5c2510cd5f5333ca16179
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java 79d8bf1e8f9798714d14e5a0336e24a148c4ca27
> lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java 3d38ddd2dfe596b348bb48807aac5caeab5e936a
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FailedAttempt.java PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java a57a6e4ed1827ee5094cbfd423c183766d9dc5f4
> lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java 1269e45ff7e5b9392108e338735b9774fb8a2686
> lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java d8e654d72989d7a8f9334adc81b82a9e0810ee39
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java 2dff9afeb384738fa21ef7381cb5a54e3c02165a
>
> Diff: https://reviews.apache.org/r/41506/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>