You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Ajay Yadava <aj...@gmail.com> on 2016/03/24 15:09:55 UTC

Review Request 45286: AlarmService for the Scheduler

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

Review request for lens.


Bugs: LENS-987
    https://issues.apache.org/jira/browse/LENS-987


Repository: lens


Description
-------

Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.


Diffs
-----

  lens-server-api/pom.xml 8304279 
  lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
  lens-server/pom.xml 75b8b32 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
  lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
  lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
  pom.xml 06cae12 

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


Testing
-------


Thanks,

Ajay Yadava


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Ajay Yadava <aj...@gmail.com>.

> On April 3, 2016, 7:01 a.m., Raju Bairishetti wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 65
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line65>
> >
> >     Looks like, service is not added to HealthCheckerRegistry. Can you add?

Ah, you are right! Thanks for catching it, fixed :)


> On April 3, 2016, 7:01 a.m., Raju Bairishetti wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 74
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line74>
> >
> >     Should we throw run time exception here instead of ignoring exception?
> >     
> >     I think at least we should change the health status to identify whether service is started or not.

Updated the health status if the service fails to initialize. 

I am not sure about the side effects of throwing RuntimeException, if you feel that is the correct way to handle failures instead of changing the health status then please let me know and I will make changes accordingly.


> On April 3, 2016, 7:01 a.m., Raju Bairishetti wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 83
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line83>
> >
> >     +1 for changing the health status for start() and init() failures

Done.


- Ajay


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


On April 11, 2016, 8:44 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 8:44 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 309921f 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Raju Bairishetti <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/#review126715
-----------------------------------------------------------




lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 65)
<https://reviews.apache.org/r/45286/#comment189759>

    Looks like, service is not added to HealthCheckerRegistry. Can you add?



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 74)
<https://reviews.apache.org/r/45286/#comment189760>

    Should we throw run time exception here instead of ignoring exception?
    
    I think at least we should change the health status to identify whether service is started or not.



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 83)
<https://reviews.apache.org/r/45286/#comment189758>

    +1 for changing the health status for start() and init() failures


- Raju Bairishetti


On March 24, 2016, 2:09 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 2:09 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 06cae12 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/#review128371
-----------------------------------------------------------




lens-server/pom.xml (line 235)
<https://reviews.apache.org/r/45286/#comment191806>

    Please move the version definition to main pom's dependency management section



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 77)
<https://reviews.apache.org/r/45286/#comment191807>

    Text seems wrong - Should say 'Alarm service'



lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java (line 90)
<https://reviews.apache.org/r/45286/#comment191808>

    Starting all services but stopping only alarm service. 
    
    Can you start only alarm service and event service, instead of starting all ?


- Amareshwari Sriramadasu


On April 11, 2016, 8:44 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 8:44 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 309921f 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/#review129282
-----------------------------------------------------------


Ship it!




Ship It!

- Amareshwari Sriramadasu


On April 13, 2016, 2:21 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated April 13, 2016, 2:21 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 06cae12 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/#review130061
-----------------------------------------------------------




lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java (line 77)
<https://reviews.apache.org/r/45286/#comment193702>

    Seeing test failures


- Amareshwari Sriramadasu


On April 13, 2016, 2:21 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated April 13, 2016, 2:21 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 06cae12 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/
-----------------------------------------------------------

(Updated April 13, 2016, 2:21 p.m.)


Review request for lens.


Changes
-------

Addressed the review comments.


Bugs: LENS-987
    https://issues.apache.org/jira/browse/LENS-987


Repository: lens


Description
-------

Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.


Diffs (updated)
-----

  lens-server-api/pom.xml 8304279 
  lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
  lens-server/pom.xml 75b8b32 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
  lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
  lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
  lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
  pom.xml 06cae12 

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


Testing
-------


Thanks,

Ajay Yadava


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/
-----------------------------------------------------------

(Updated April 11, 2016, 8:44 a.m.)


Review request for lens.


Bugs: LENS-987
    https://issues.apache.org/jira/browse/LENS-987


Repository: lens


Description
-------

Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.


Diffs (updated)
-----

  lens-server-api/pom.xml 8304279 
  lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
  lens-server/pom.xml 75b8b32 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
  lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
  lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
  lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
  pom.xml 309921f 

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


Testing
-------


Thanks,

Ajay Yadava


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Ajay Yadava <aj...@gmail.com>.

> On March 31, 2016, 10:52 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 83
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line83>
> >
> >     Should health status be changed in these cases?

Fixed.


> On March 31, 2016, 10:52 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 97
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line97>
> >
> >     Can you doc about this method? 
> >     
> >     Who/when will we call this method ?

Documented the required details.


> On March 31, 2016, 10:52 a.m., Amareshwari Sriramadasu wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java, line 48
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313698#file1313698line48>
> >
> >     Is event id same as jobhandle?

Yes, that is what I intended, do you see an issue with this approach?


> On March 31, 2016, 10:52 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 111
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line111>
> >
> >     Should this be schedule name?

It does not matter. This is used by Quartz to group various requests across applications. To trace the origin and keep the service general I have renamed the group to "AlarmService". This will help in identifying from where the service was registered in case of multiple applications registering for triggers.


- Ajay


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


On April 11, 2016, 8:44 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 8:44 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 309921f 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/#review126292
-----------------------------------------------------------




lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java (line 48)
<https://reviews.apache.org/r/45286/#comment189246>

    Is event id same as jobhandle?



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 83)
<https://reviews.apache.org/r/45286/#comment189249>

    Should health status be changed in these cases?



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 97)
<https://reviews.apache.org/r/45286/#comment189251>

    Can you doc about this method? 
    
    Who/when will we call this method ?



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 111)
<https://reviews.apache.org/r/45286/#comment189248>

    Should this be schedule name?


- Amareshwari Sriramadasu


On March 24, 2016, 2:09 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 2:09 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 06cae12 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Ajay Yadava <aj...@gmail.com>.

> On March 28, 2016, 3:42 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/LensServices.java, line 194
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313700#file1313700line194>
> >
> >     Do we need to start Alarm Service separateley or should it be started as part of scheduler service?
> >     
> >     In future schedule service will suppoport more triggers apart form time trigger. In that case it will be better if we don't start those Triggers separately.

There is no side effect of starting the triggers separately and they are generic enough that there can be more consumers in future other than the scheduler. Hece I think the start should be kept separate.


> On March 28, 2016, 3:42 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 49
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line49>
> >
> >     Not sure of the name AlaramService. Alarm generally refers to an alarming situation. Should we change it to something else (like TimeScheduler, TimeSchedulerService, TimeTriggerScheduler, TimeTriggerService)?

I think in programming usually Alert is the term for such situations. I did a search on "java quartz alarm" and it seems to be a common usage. Just being lazy as I have other patches done on top of this patch, if you feel strongly about it, I will change it before the final patch for scheduler.


> On March 28, 2016, 3:42 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 181
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line181>
> >
> >     What happens if server restarts after notifying the schedule event. Will/Should the schedule service retrigger this schedule on startup?

Good question. I intend to register again, other option will involve all such services to maintain the state, so planning to keep it in one place and register again.


> On March 28, 2016, 3:42 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 128
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line128>
> >
> >     This can be a checked exception and not a  RunTimeExecption.

Yes, that will be better. Made the change, thanks for the suggestion.


> On March 28, 2016, 3:42 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java, line 163
> > <https://reviews.apache.org/r/45286/diff/1/?file=1313701#file1313701line163>
> >
> >     Can this method tell the status of unSchedule either via a boolean result or execption to its caller.

Excellent suggestion, made the changes!


- Ajay


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


On April 11, 2016, 8:44 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 8:44 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java 6852265 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 309921f 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Re: Review Request 45286: AlarmService for the Scheduler

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45286/#review125588
-----------------------------------------------------------




lens-server/src/main/java/org/apache/lens/server/LensServices.java (line 194)
<https://reviews.apache.org/r/45286/#comment188464>

    Do we need to start Alarm Service separateley or should it be started as part of scheduler service?
    
    In future schedule service will suppoport more triggers apart form time trigger. In that case it will be better if we don't start those Triggers separately.



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 49)
<https://reviews.apache.org/r/45286/#comment188463>

    Not sure of the name AlaramService. Alarm generally refers to an alarming situation. Should we change it to something else (like TimeScheduler, TimeSchedulerService, TimeTriggerScheduler, TimeTriggerService)?



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 128)
<https://reviews.apache.org/r/45286/#comment188465>

    This can be a checked exception and not a  RunTimeExecption.



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 163)
<https://reviews.apache.org/r/45286/#comment188466>

    Can this method tell the status of unSchedule either via a boolean result or execption to its caller.



lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java (line 181)
<https://reviews.apache.org/r/45286/#comment188467>

    What happens if server restarts after notifying the schedule event. Will/Should the schedule service retrigger this schedule on startup?


- Puneet Gupta


On March 24, 2016, 2:09 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45286/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 2:09 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-987
>     https://issues.apache.org/jira/browse/LENS-987
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Alarm service will provide time notifications to the scheduler to trigger the queries on a specified schedule. The schedule is built using the frequency.
> 
> 
> Diffs
> -----
> 
>   lens-server-api/pom.xml 8304279 
>   lens-server-api/src/main/java/org/apache/lens/server/api/events/SchedulerAlarmEvent.java PRE-CREATION 
>   lens-server/pom.xml 75b8b32 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 
>   lens-server/src/main/java/org/apache/lens/server/scheduler/notification/services/AlarmService.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/scheduler/notification/services/AlarmServiceTest.java PRE-CREATION 
>   pom.xml 06cae12 
> 
> Diff: https://reviews.apache.org/r/45286/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>