feat: introduce timeout and retry parameter for BM executor shutdown#1113
Conversation
| if (root != null && NodeModelUtils.getNode(root) instanceof ICompositeNode composite) { | ||
| composite.getText(); // side-effect on removing com.avaloq.tools.ddk.xtext.resource.persistence.ProxyCompositeNode | ||
| if (root != null) { | ||
| NodeModelUtils.getNode(root).getText(); // side-effect on removing com.avaloq.tools.ddk.xtext.resource.persistence.ProxyCompositeNode |
There was a problem hiding this comment.
don't we need to check for null on NodeModelUtils.getNode(root)?
|
|
||
| // Attempt to wait for queued work to complete | ||
| int retries = 0; | ||
| boolean terminated = false; |
There was a problem hiding this comment.
| boolean terminated = false; | |
| boolean terminated; |
| queuedTaskCount, activeTaskCount, binaryStorageExecutor.getQueue().size(), binaryStorageExecutor.getActiveCount())); | ||
| binaryStorageExecutor.shutdownNow(); | ||
| } catch (InterruptedException e) { | ||
| LOGGER.warn("Interrupted waiting for binaryStorageExecutor shutdown, terminating"); |
There was a problem hiding this comment.
I think it would be nice to keep the part of the message "tasks not completed in time, start with %d queued / %d active; now have %d / %d" when interrupted.
The awaitBinaryStorageExecutorTermination() in MonitoredClusteringBuilderState is extended with parameters that allow the timeout for the shutdown and the number of attempts to make to be specified. (also implement 2 small cleanups of code causing a compile error depending on settings and a FB warning suppression that is no longer necessary)
| public static void ensureNodeModelLoaded(final Resource resource) { | ||
| if (resource instanceof LazyLinkingResource2 lazyLinkinResource && lazyLinkinResource.isLoadedFromStorage()) { | ||
| EObject root = resource.getContents().get(0); | ||
| if (root != null && NodeModelUtils.getNode(root) instanceof ICompositeNode composite) { |
There was a problem hiding this comment.
Your version is fine for me, but what about just naming the variable rootNode instead of composite? That would be a smaller change than this one.
There was a problem hiding this comment.
It wouldn't fix the compiler error I see when I include the DDK in an ASMD workspace. The compiler complains because NodeModelUtils.getNode() returns an ICompositeNode, so the instanceof test is always true. The message is "Expression type cannot be a subtype of the Pattern type".
2 participants
The awaitBinaryStorageExecutorTermination() in
MonitoredClusteringBuilderState is extended with parameters that allow the timeout for the shutdown and the number of attempts to make to be specified.
(also implement 2 small cleanups of code causing a compile error depending on settings and a FB warning suppression that is no longer necessary)