Reamer commented on code in PR #4382:
URL: https://github.com/apache/zeppelin/pull/4382#discussion_r903486737
##########
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:
As far as I can see, it does. We also have @PreDestroy in the Lucene
sub-system to close the index cleanly and shut down the SearchService cleanly
with it.
https://github.com/apache/zeppelin/blob/5ebd0fe6231ec0b6d3576ee4ec3be14449fe5c2f/zeppelin-zengine/src/main/java/org/apache/zeppelin/search/LuceneSearch.java#L416-L430
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]