From c2b931a5a0ea81bdc93db1f0cf632b07717c5066 Mon Sep 17 00:00:00 2001 From: Mateusz Waronwy Date: Fri, 13 Nov 2020 16:51:38 +0100 Subject: [PATCH] JAL-3690 separate startup and poll code in SeqAnnotationCalcWorker --- src/jalview/api/PollableAlignCalcWorkerI.java | 2 +- src/jalview/gui/AlignFrame.java | 6 + src/jalview/gui/Desktop.java | 7 + src/jalview/gui/IProgressIndicator.java | 7 + src/jalview/gui/PCAPanel.java | 6 + src/jalview/gui/ProgressBar.java | 17 ++ src/jalview/gui/StructureChooser.java | 6 + src/jalview/gui/WebserviceInfo.java | 6 + src/jalview/workers/AlignCalcManager.java | 2 +- src/jalview/workers/AlignCalcManager2.java | 19 +- .../ws/jws2/SeqAnnotationServiceCalcWorker.java | 301 ++++++++------------ 11 files changed, 190 insertions(+), 189 deletions(-) diff --git a/src/jalview/api/PollableAlignCalcWorkerI.java b/src/jalview/api/PollableAlignCalcWorkerI.java index 9697187..25074ec 100644 --- a/src/jalview/api/PollableAlignCalcWorkerI.java +++ b/src/jalview/api/PollableAlignCalcWorkerI.java @@ -14,5 +14,5 @@ public interface PollableAlignCalcWorkerI extends AlignCalcWorkerI public void cancel(); - public void cleanUp(); + public void done(); } diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java index 9e0fe6f..cc81eef 100644 --- a/src/jalview/gui/AlignFrame.java +++ b/src/jalview/gui/AlignFrame.java @@ -991,6 +991,12 @@ public class AlignFrame extends GAlignFrame { progressBar.setProgressBar(message, id); } + + @Override + public void removeProgressBar(long id) + { + progressBar.removeProgressBar(id); + } @Override public void registerHandler(final long id, diff --git a/src/jalview/gui/Desktop.java b/src/jalview/gui/Desktop.java index 63b621e..e9bf09f 100644 --- a/src/jalview/gui/Desktop.java +++ b/src/jalview/gui/Desktop.java @@ -2520,6 +2520,13 @@ public class Desktop extends jalview.jbgui.GDesktop progressBars.put(Long.valueOf(id), addProgressPanel(message)); } } + + @Override + public void removeProgressBar(long id) + { + //TODO + throw new UnsupportedOperationException("not implemented"); + } /* * (non-Javadoc) diff --git a/src/jalview/gui/IProgressIndicator.java b/src/jalview/gui/IProgressIndicator.java index 35bd871..c250aca 100644 --- a/src/jalview/gui/IProgressIndicator.java +++ b/src/jalview/gui/IProgressIndicator.java @@ -56,4 +56,11 @@ public interface IProgressIndicator */ boolean operationInProgress(); + /** + * Remove progress bar with a given id from the panel. + * + * @param id + */ + void removeProgressBar(long id); + } diff --git a/src/jalview/gui/PCAPanel.java b/src/jalview/gui/PCAPanel.java index 96a8b0d..a2114f0 100644 --- a/src/jalview/gui/PCAPanel.java +++ b/src/jalview/gui/PCAPanel.java @@ -609,6 +609,12 @@ public class PCAPanel extends GPCAPanel // // setMenusForViewport(); // validate(); } + + @Override + public void removeProgressBar(long id) + { + progressBar.removeProgressBar(id); + } @Override public void registerHandler(final long id, diff --git a/src/jalview/gui/ProgressBar.java b/src/jalview/gui/ProgressBar.java index 011d810..abf096f 100644 --- a/src/jalview/gui/ProgressBar.java +++ b/src/jalview/gui/ProgressBar.java @@ -166,6 +166,23 @@ public class ProgressBar implements IProgressIndicator }); } + + @Override + public void removeProgressBar(final long id) + { + SwingUtilities.invokeLater(() -> { + JPanel progressPanel = progressBars.get(id); + if (progressPanel != null) + { + progressBars.remove(id); + if (progressBarHandlers.containsKey(id)) + { + progressBarHandlers.remove(id); + } + removeRow(progressPanel); + } + }); + } /** * Lays out progress bar container hierarchy diff --git a/src/jalview/gui/StructureChooser.java b/src/jalview/gui/StructureChooser.java index 33d8c33..3807ebd 100644 --- a/src/jalview/gui/StructureChooser.java +++ b/src/jalview/gui/StructureChooser.java @@ -1386,6 +1386,12 @@ public class StructureChooser extends GStructureChooser { progressBar.setProgressBar(message, id); } + + @Override + public void removeProgressBar(long id) + { + progressBar.removeProgressBar(id); + } @Override public void registerHandler(long id, IProgressIndicatorHandler handler) diff --git a/src/jalview/gui/WebserviceInfo.java b/src/jalview/gui/WebserviceInfo.java index 25ade21..03d76ee 100644 --- a/src/jalview/gui/WebserviceInfo.java +++ b/src/jalview/gui/WebserviceInfo.java @@ -928,6 +928,12 @@ public void hyperlinkUpdate(HyperlinkEvent e) { progressBar.setProgressBar(message, id); } + + @Override + public void removeProgressBar(long id) + { + progressBar.removeProgressBar(id); + } @Override public void registerHandler(final long id, diff --git a/src/jalview/workers/AlignCalcManager.java b/src/jalview/workers/AlignCalcManager.java index 79ca863..9f8d4e8 100644 --- a/src/jalview/workers/AlignCalcManager.java +++ b/src/jalview/workers/AlignCalcManager.java @@ -185,7 +185,7 @@ public class AlignCalcManager implements AlignCalcManagerI try { worker.run(); - } catch (Exception e) + } catch (Throwable e) { e.printStackTrace(); } diff --git a/src/jalview/workers/AlignCalcManager2.java b/src/jalview/workers/AlignCalcManager2.java index 7a19321..0057500 100644 --- a/src/jalview/workers/AlignCalcManager2.java +++ b/src/jalview/workers/AlignCalcManager2.java @@ -183,6 +183,8 @@ public class AlignCalcManager2 implements AlignCalcManagerI2 } else if (!completed) { + Cache.log.debug(format("Polling worker %s", + worker.getClass().getName())); if (worker.poll()) { Cache.log.debug(format("Worker %s finished", @@ -198,14 +200,12 @@ public class AlignCalcManager2 implements AlignCalcManagerI2 } if (completed) { - try - { - future.cancel(false); - } - catch (NullPointerException ignored) - { - // extremely unlikely to happen - } + Cache.log.debug(format("Finalizing completed worker %s", + worker.getClass().getName())); + worker.done(); + // almost impossible, but the future may be null at this point + // let it throw NPE to cancel forcefully + future.cancel(false); } } }; @@ -224,6 +224,9 @@ public class AlignCalcManager2 implements AlignCalcManagerI2 task.cancel(false); executor.submit(() -> { worker.cancel(); + Cache.log.debug(format("Finalizing cancelled worker %s", + worker.getClass().getName())); + worker.done(); }); } } diff --git a/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java b/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java index ae03dfb..53b790d 100644 --- a/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java +++ b/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java @@ -26,6 +26,7 @@ import jalview.analysis.SeqsetUtils; import jalview.api.AlignViewportI; import jalview.api.AlignmentViewPanel; import jalview.api.FeatureColourI; +import jalview.api.PollableAlignCalcWorkerI; import jalview.bin.Cache; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; @@ -64,7 +65,7 @@ import java.util.List; import java.util.Map; public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker - implements WSAnnotationCalcManagerI + implements WSAnnotationCalcManagerI, PollableAlignCalcWorkerI { protected ServiceWithParameters service; @@ -162,17 +163,11 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker return gapMap; } - public SeqAnnotationServiceCalcWorker(AlignViewportI alignViewport, - AlignmentViewPanel alignPanel) - { - super(alignViewport, alignPanel); - } - public SeqAnnotationServiceCalcWorker(ServiceWithParameters service, AlignFrame alignFrame, WsParamSetI preset, List paramset) { - this(alignFrame.getCurrentView(), alignFrame.alignPanel); + super(alignFrame.getCurrentView(), alignFrame.alignPanel); // TODO: both these fields needed ? this.alignFrame = alignFrame; this.guiProgress = alignFrame; @@ -185,6 +180,7 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker .getEndpoint(); } catch (ClassCastException cce) { + annotService = null; JvOptionPane.showMessageDialog(Desktop.desktop, MessageManager.formatMessage( "label.service_called_is_not_an_annotation_service", @@ -194,6 +190,7 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker JvOptionPane.WARNING_MESSAGE); } + cancellable = CancellableI.class.isInstance(annotService); // configure submission flags proteinAllowed = service.isProteinService(); nucleotidesAllowed = service.isNucleotideService(); @@ -218,7 +215,8 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker return annotService != null; } - protected jalview.ws.api.SequenceAnnotationServiceI annotService = null; + protected SequenceAnnotationServiceI annotService; + protected final boolean cancellable; volatile JobId rslt = null; @@ -226,8 +224,13 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker private int min_valid_seqs; - @Override - public void run() throws Exception + + private long progressId = -1; + JobStateSummary job = null; + WebserviceInfo info = null; + List seqs = null; + + @Override public void startUp() throws Throwable { if (alignViewport.isClosed()) { @@ -239,134 +242,106 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker return; } - long progressId = -1; - - int serverErrorsLeft = 3; - final boolean cancellable = CancellableI.class - .isAssignableFrom(annotService.getClass()); StringBuffer msg = new StringBuffer(); - JobStateSummary job = new JobStateSummary(); - WebserviceInfo info = new WebserviceInfo("foo", "bar", false); - try - { - List seqs = getInputSequences( - alignViewport.getAlignment(), - bySequence ? alignViewport.getSelectionGroup() : null); + job = new JobStateSummary(); + info = new WebserviceInfo("foo", "bar", false); - if (seqs == null || !checkValidInputSeqs(seqs)) - { - jalview.bin.Cache.log.debug( - "Sequences for analysis service were null or not valid"); - return; - } + seqs = getInputSequences( + alignViewport.getAlignment(), + bySequence ? alignViewport.getSelectionGroup() : null); - if (guiProgress != null) - { - guiProgress.setProgressBar(service.getActionText(), - progressId = System.currentTimeMillis()); - } - jalview.bin.Cache.log.debug("submitted " + seqs.size() - + " sequences to " + service.getActionText()); + if (seqs == null || !checkValidInputSeqs(seqs)) + { + jalview.bin.Cache.log.debug( + "Sequences for analysis service were null or not valid"); + return; + } - rslt = annotService.submitToService(seqs, getPreset(), - getArguments()); - if (rslt == null) - { - return; - } - // TODO: handle job submission error reporting here. - Cache.log.debug("Service " + service.getUri() + "\nSubmitted job ID: " - + rslt); - ; - // /// - // otherwise, construct WsJob and any UI handlers - running = new AnnotationWsJob(); - running.setJobHandle(rslt); - running.setSeqNames(seqNames); - running.setStartPos(start); - running.setSeqs(seqs); - job.updateJobPanelState(info, "", running); - if (guiProgress != null) - { - guiProgress.registerHandler(progressId, - new IProgressIndicatorHandler() - { + if (guiProgress != null) + { + guiProgress.setProgressBar(service.getActionText(), + progressId = System.currentTimeMillis()); + } + jalview.bin.Cache.log.debug("submitted " + seqs.size() + + " sequences to " + service.getActionText()); - @Override - public boolean cancelActivity(long id) - { - ((CancellableI) annotService).cancel(running); - return true; - } - - @Override - public boolean canCancel() - { - return cancellable; - } - }); - } - - // /// - // and poll for updates until job finishes, fails or becomes stale - - boolean finished = false; - do - { - Cache.log.debug("Updating status for annotation service."); - annotService.updateStatus(running); - job.updateJobPanelState(info, "", running); - if (running.isSubjobComplete()) - { - Cache.log.debug( - "Finished polling analysis service job: status reported is " - + running.getState()); - finished = true; - } - else - { - Cache.log.debug("Status now " + running.getState()); - } + rslt = annotService.submitToService(seqs, getPreset(), + getArguments()); + if (rslt == null) + { + return; + } + // TODO: handle job submission error reporting here. + Cache.log.debug("Service " + service.getUri() + "\nSubmitted job ID: " + + rslt); + ; + // /// + // otherwise, construct WsJob and any UI handlers + running = new AnnotationWsJob(); + running.setJobHandle(rslt); + running.setSeqNames(seqNames); + running.setStartPos(start); + running.setSeqs(seqs); + job.updateJobPanelState(info, "", running); + if (guiProgress != null) + { + guiProgress.registerHandler(progressId, + new IProgressIndicatorHandler() + { - // pull any stats - some services need to flush log output before - // results are available - Cache.log.debug("Updating progress log for annotation service."); + @Override + public boolean cancelActivity(long id) + { + calcMan.cancelWorker(SeqAnnotationServiceCalcWorker.this); + return true; + } - try - { - annotService.updateJobProgress(running); - } catch (Throwable thr) - { - Cache.log.debug("Ignoring exception during progress update.", - thr); - } - Cache.log.trace("Result of poll: " + running.getStatus()); + @Override + public boolean canCancel() + { + return cancellable; + } + }); + } + } + + @Override public boolean poll() throws Throwable + { + boolean finished = false; + + Cache.log.debug("Updating status for annotation service."); + annotService.updateStatus(running); + job.updateJobPanelState(info, "", running); + if (running.isSubjobComplete()) + { + Cache.log.debug( + "Finished polling analysis service job: status reported is " + + running.getState()); + finished = true; + } + else + { + Cache.log.debug("Status now " + running.getState()); + } - if (!finished && !running.isFailed()) - { - try - { - Cache.log.debug("Analysis service job thread sleeping."); - Thread.sleep(200); - Cache.log.debug("Analysis service job thread woke."); - } catch (InterruptedException x) - { - } - ; - } - } while (!finished); + // pull any stats - some services need to flush log output before + // results are available + Cache.log.debug("Updating progress log for annotation service."); + try + { + annotService.updateJobProgress(running); + } catch (Throwable thr) + { + Cache.log.debug("Ignoring exception during progress update.", + thr); + } + Cache.log.trace("Result of poll: " + running.getStatus()); + + + if (finished) + { Cache.log.debug("Job poll loop exited. Job is " + running.getState()); - // TODO: need to poll/retry - if (serverErrorsLeft > 0) - { - try - { - Thread.sleep(200); - } catch (InterruptedException x) - { - } - } if (running.isFinished()) { // expect there to be results to collect @@ -466,60 +441,28 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker } } Cache.log.debug("Annotation Service Worker thread finished."); - } -// TODO: use service specitic exception handlers -// catch (JobSubmissionException x) -// { -// -// System.err.println( -// "submission error with " + getServiceActionText() + " :"); -// x.printStackTrace(); -// calcMan.disableWorker(this); -// } catch (ResultNotAvailableException x) -// { -// System.err.println("collection error:\nJob ID: " + rslt); -// x.printStackTrace(); -// calcMan.disableWorker(this); -// -// } catch (OutOfMemoryError error) -// { -// calcMan.disableWorker(this); -// -// ap.raiseOOMWarning(getServiceActionText(), error); -// } - catch (Throwable x) - { - calcMan.disableWorker(this); - System.err - .println("Blacklisting worker due to unexpected exception:"); - x.printStackTrace(); - throw new Exception(x); - } finally + } + + return finished; + } + + @Override public void cancel() + { + cancelCurrentJob(); + } + + @Override public void done() + { + if (ap != null) { - - if (ap != null) + if (guiProgress != null && progressId != -1) { - if (guiProgress != null && progressId != -1) - { - guiProgress.setProgressBar("", progressId); - } - // TODO: may not need to paintAlignment again ! - ap.paintAlignment(false, false); - } - if (msg.length() > 0) - { - // TODO: stash message somewhere in annotation or alignment view. - // code below shows result in a text box popup - /* - * jalview.gui.CutAndPasteTransfer cap = new - * jalview.gui.CutAndPasteTransfer(); cap.setText(msg.toString()); - * jalview.gui.Desktop.addInternalFrame(cap, - * "Job Status for "+getServiceActionText(), 600, 400); - */ + guiProgress.removeProgressBar(progressId); } + // TODO: may not need to paintAlignment again ! + ap.paintAlignment(false, false); } - } /** @@ -552,7 +495,7 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker try { String id = running.getJobId(); - if (((CancellableI) annotService).cancel(running)) + if (cancellable && ((CancellableI) annotService).cancel(running)) { System.err.println("Cancelled job " + id); } -- 1.7.10.2