Reamer commented on code in PR #4382:
URL: https://github.com/apache/zeppelin/pull/4382#discussion_r903619482
##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/scheduler/QuartzSchedulerService.java:
##########
@@ -51,123 +51,110 @@ public class QuartzSchedulerService implements
SchedulerService {
private final ZeppelinConfiguration zeppelinConfiguration;
private final Notebook notebook;
private final Scheduler scheduler;
- private final Thread loadingNotesThread;
@Inject
public QuartzSchedulerService(ZeppelinConfiguration zeppelinConfiguration,
Notebook notebook)
throws SchedulerException {
this.zeppelinConfiguration = zeppelinConfiguration;
this.notebook = notebook;
this.scheduler = getScheduler();
- this.scheduler.getListenerManager().addJobListener(new CronJobListener());
+ this.scheduler.getListenerManager().addJobListener(new
MetricCronJobListener());
+ this.scheduler.getListenerManager().addTriggerListener(new
ZeppelinCronJobTriggerListerner());
this.scheduler.start();
-
- // Do in a separated thread because there may be many notes,
- // loop all notes in the main thread may block the restarting of Zeppelin
server
- // TODO(zjffdu) It may cause issue when user delete note before this
thread is finished
- this.loadingNotesThread = new Thread(() -> {
- LOGGER.info("Starting init cronjobs");
- notebook.getNotesInfo().stream()
- .forEach(entry -> {
- try {
- refreshCron(entry.getId());
- } catch (Exception e) {
- LOGGER.warn("Fail to refresh cron for note: {}",
entry.getId());
- }
- });
- LOGGER.info("Complete init cronjobs");
- });
- loadingNotesThread.setName("Init CronJob Thread");
- loadingNotesThread.setDaemon(true);
- loadingNotesThread.start();
+ // Start Init
+ notebook.addInitConsumer(this::refreshCron);
}
+
private Scheduler getScheduler() throws SchedulerException {
return new StdSchedulerFactory().getScheduler();
}
- /**
- * This is only for testing, unit test should always call this method in
setup() before testing.
- */
- @VisibleForTesting
- public void waitForFinishInit() {
+ @PreDestroy
Review Comment:
You made me curious with your statement and it seems @PreDestroy is not
working properly.
I will investigate this further and remove the change from this PullRequest
for now. The close method is only cosmetic and was needed when I took a
different approach.
Initial investigation has shown that `sharedServiceLocator.shutdown();` is
needed to trigger the code with the PreDestroy annotation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
