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

Reply via email to