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 2016/08/26 11:28:16 UTC
Review Request 51457: Handle Restart
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-1286
https://issues.apache.org/jira/browse/LENS-1286
Repository: lens
Description
-------
When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
If the instance is in RUNNING state, it will check the query status and update accordingly.
P.S. Ignore ResourceTest file. It is in separate review request.
Diffs
-----
lens-api/src/main/java/org/apache/lens/api/scheduler/SchedulerJobInstanceRun.java 8532ed0
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 323e053
lens-server/src/test/java/org/apache/lens/server/scheduler/ScheduleResourceTest.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
Diff: https://reviews.apache.org/r/51457/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 51457: Handle Restart
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/#review148304
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java (line 225)
<https://reviews.apache.org/r/51457/#comment215784>
Can you do slf4j parameterized logging?
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java (line 50)
<https://reviews.apache.org/r/51457/#comment215785>
Put back the groups annotation.
- Amareshwari Sriramadasu
On Sept. 8, 2016, 12:16 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51457/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2016, 12:16 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1286
> https://issues.apache.org/jira/browse/LENS-1286
>
>
> Repository: lens
>
>
> Description
> -------
>
> When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
> If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
> If the instance is in RUNNING state, it will check the query status and update accordingly.
>
> P.S. Ignore ResourceTest file. It is in separate review request.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 9cee0c2
> lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
> lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
>
> Diff: https://reviews.apache.org/r/51457/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 51457: Handle Restart
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/
-----------------------------------------------------------
(Updated Sept. 12, 2016, 6:53 a.m.)
Review request for lens.
Bugs: LENS-1286
https://issues.apache.org/jira/browse/LENS-1286
Repository: lens
Description
-------
When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
If the instance is in RUNNING state, it will check the query status and update accordingly.
P.S. Ignore ResourceTest file. It is in separate review request.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 9cee0c2
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
Diff: https://reviews.apache.org/r/51457/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 51457: Handle Restart
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/
-----------------------------------------------------------
(Updated Sept. 8, 2016, 12:16 p.m.)
Review request for lens.
Bugs: LENS-1286
https://issues.apache.org/jira/browse/LENS-1286
Repository: lens
Description
-------
When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
If the instance is in RUNNING state, it will check the query status and update accordingly.
P.S. Ignore ResourceTest file. It is in separate review request.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 9cee0c2
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
Diff: https://reviews.apache.org/r/51457/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 51457: Handle Restart
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/
-----------------------------------------------------------
(Updated Sept. 8, 2016, 12:02 p.m.)
Review request for lens.
Bugs: LENS-1286
https://issues.apache.org/jira/browse/LENS-1286
Repository: lens
Description
-------
When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
If the instance is in RUNNING state, it will check the query status and update accordingly.
P.S. Ignore ResourceTest file. It is in separate review request.
Diffs (updated)
-----
lens-api/src/main/java/org/apache/lens/api/scheduler/SchedulerJobInstanceRun.java 8532ed0
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 9cee0c2
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
Diff: https://reviews.apache.org/r/51457/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 51457: Handle Restart
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/#review146939
-----------------------------------------------------------
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java (line 49)
<https://reviews.apache.org/r/51457/#comment213813>
This should have a test-group.
lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java (line 107)
<https://reviews.apache.org/r/51457/#comment213814>
Is this kind of service restart safe?
- Lavkesh Lahngir
On Aug. 26, 2016, 11:28 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51457/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2016, 11:28 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1286
> https://issues.apache.org/jira/browse/LENS-1286
>
>
> Repository: lens
>
>
> Description
> -------
>
> When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
> If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
> If the instance is in RUNNING state, it will check the query status and update accordingly.
>
> P.S. Ignore ResourceTest file. It is in separate review request.
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/scheduler/SchedulerJobInstanceRun.java 8532ed0
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 323e053
> lens-server/src/test/java/org/apache/lens/server/scheduler/ScheduleResourceTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
> lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
>
> Diff: https://reviews.apache.org/r/51457/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 51457: Handle Restart
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On Sept. 8, 2016, 9:32 a.m., Rajat Khandelwal wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java, line 479
> > <https://reviews.apache.org/r/51457/diff/1/?file=1486693#file1486693line479>
> >
> > Log seems misplaced. We're not killing here, right?
Yes, we are killing the instance. Because there is no query to kill, we can just update the status and move on.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/#review148004
-----------------------------------------------------------
On Aug. 26, 2016, 11:28 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51457/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2016, 11:28 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1286
> https://issues.apache.org/jira/browse/LENS-1286
>
>
> Repository: lens
>
>
> Description
> -------
>
> When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
> If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
> If the instance is in RUNNING state, it will check the query status and update accordingly.
>
> P.S. Ignore ResourceTest file. It is in separate review request.
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/scheduler/SchedulerJobInstanceRun.java 8532ed0
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 323e053
> lens-server/src/test/java/org/apache/lens/server/scheduler/ScheduleResourceTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
> lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
>
> Diff: https://reviews.apache.org/r/51457/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 51457: Handle Restart
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51457/#review148004
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java (line 144)
<https://reviews.apache.org/r/51457/#comment215354>
hardcoding "admin" might not work in some production scenarios. Alternatives:
1. Keep it configurable on server level
2. Get the user from the job definition and open as that user.
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java (line 152)
<https://reviews.apache.org/r/51457/#comment215355>
Let's mention instance and job too like on line 140
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java (line 159)
<https://reviews.apache.org/r/51457/#comment215356>
logger here?
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java (line 206)
<https://reviews.apache.org/r/51457/#comment215357>
let's add a warning log here.
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java (line 475)
<https://reviews.apache.org/r/51457/#comment215359>
Log seems misplaced. We're not killing here, right?
lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java (line 478)
<https://reviews.apache.org/r/51457/#comment215358>
remove first `with`
- Rajat Khandelwal
On Aug. 26, 2016, 4:58 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51457/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2016, 4:58 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1286
> https://issues.apache.org/jira/browse/LENS-1286
>
>
> Repository: lens
>
>
> Description
> -------
>
> When the server is restarted, it reads all instances which where in WAITING, LAUNCHED or RUNNING states.
> If the instance is in WAITING or LAUNCHED state, it is killed and it is rerun.
> If the instance is in RUNNING state, it will check the query status and update accordingly.
>
> P.S. Ignore ResourceTest file. It is in separate review request.
>
>
> Diffs
> -----
>
> lens-api/src/main/java/org/apache/lens/api/scheduler/SchedulerJobInstanceRun.java 8532ed0
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 966a64e
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerQueryEventListener.java 077d531
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerServiceImpl.java 323e053
> lens-server/src/test/java/org/apache/lens/server/scheduler/ScheduleResourceTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/SchedulerRestartTest.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/scheduler/TestSchedulerServiceImpl.java 130df5f
> lens-server/src/test/java/org/apache/lens/server/scheduler/util/SchedulerTestUtils.java a36b2aa
>
> Diff: https://reviews.apache.org/r/51457/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>