[MGNLWORKFLOW-317] Upon changing configuration of workflow manager, no workflow can be executed until restart Created: 30/Dec/15  Updated: 19/Apr/16  Resolved: 10/Mar/16

Status: Closed
Project: Magnolia Workflow Module
Component/s: None
Affects Version/s: 5.5.1
Fix Version/s: 5.5.2

Type: Bug Priority: Neutral
Reporter: Jan Haderka Assignee: Federico Grilli
Resolution: Fixed Votes: 0
Labels: support
Remaining Estimate: 0d
Time Spent: 4.5h
Original Estimate: Not Specified

Attachments: Java Source File WorkflowBaseModule.java     File magnolia-module-workflow-5.5.2-SNAPSHOT.jar     Text File stacktrace.txt    
Issue Links:
causality
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Documentation update required:
Yes
Date of First Response:
Sprint: Basel 34
Story Points: 3

 Description   

Timebox: 1h for finding an easy solution! Otherwise we might have to invest more time

To reproduce:

  1. try to publish any page to confirm all is working and workflow is enabled
  2. go to /modules/workflow/config/workflowManager and change implementation class or simply add a property
  3. try to publish same page again ==> fails w/ exception attached

Upon adding/editing property, workflow module is restarted which triggers restart of the engine. However something there doesn't deal w/ restart properly.

Chosen solution

Eventually it was decided not to touch the workflow engine during module reloads cause that would lead it into a broken, no longer working state. Instead we log a message about the need for restarting Magnolia to account for workflow configuration changes.



 Comments   
Comment by Richard Gange [ 30/Dec/15 ]

Digging a little into this I can see the following facts. There is only one runtime manger. The MgnlRuntimeManager. You can initialize it and you can close it. Once closed it can't be reopened. I don't see any methods to do so. It does not appear that calling init() again reopens it.

The MgnlRuntimeManager is inspired by the SingletonRuntimeManager so we can't initialize another instance. I'm just wondering what is the use case to ever stop the engine unless magnolia is shutting down. At this point you can't make a configuration change anyway to a running engine. Maybe we need to look at the phase and only shut the engine down during a system shut down. Otherwise leave it running but WARN in the log that configuration changes require a restart! so that users know this.

Comment by Richard Gange [ 30/Dec/15 ]
@Override
    public void stop(ModuleLifecycleContext context) {
        try {
            if (context.getPhase() == ModuleLifecycleContext.PHASE_SYSTEM_SHUTDOWN) {
                log.info("System shutdown detected. Stopping workflow manager.");
                getWorkflowManager().stopEngine();
            }
            else log.warn("Configuration changes require a restart of the system to be loaded" + 
                " into the workflow engine! Workflow manager still running.");

        } catch (Exception ex) {
            log.error("Workflow manager failed to stop", ex);
        }
    }
Comment by Jan Haderka [ 30/Dec/15 ]

As far as I can see, there's many more things that are closed than just the engine. Environment as well, timer and scheduler services are deregistered on stop and so on.

IMO after the engine has been stopped, the workflow manager instance should be destroyed and on next call to start, new instance should be created rather than attempting to restart the previously stopped one.

Comment by Richard Gange [ 30/Dec/15 ]

rather than attempting to restart the previously stopped one.

You couldn't if you wanted to too.

the workflow manager instance should be destroyed

I see where you're going with this but it's not a good option. We don't want the system trying to reinitialize the workflow engine every time someone changes a node. I believe in this case it's a little more overhead than just a node2bean transfer. We simply cannot reconfigure the engine once it's initialized (with our current implementation anyways).

So I'm saying why stop the engine, unless the system is stopping.

Environment as well, timer and scheduler services are deregistered on stop and so on.

This isn't being done by workflow. That's observation kicking in after a configuration change.

Comment by Richard Gange [ 30/Dec/15 ]

I added a patched version of the workflow module which fixes this issue. The only changes made where to the class WorkflowBaseModule, also attached.

Comment by Jan Haderka [ 30/Dec/15 ]

I see where you're going with this but it's not a good option. We don't want the system trying to reinitialize the workflow engine every time someone changes a node.

What makes you think that we don't want to reinitialise engine? If you make changes in the node, means you have reconfigured workflow and want those changes to be picked up. Why you NOT want to do that at runtime and force user to restart whole instance instead?

I believe in this case it's a little more overhead than just a node2bean transfer.

2015-12-30 00:50:01,156 INFO  info.magnolia.module.ModuleManagerImpl            : Initializing module workflow
2015-12-30 00:50:01,235 INFO  info.magnolia.module.ModuleManagerImpl            : Starting module workflow
2015-12-30 00:50:01,235 INFO  info.magnolia.module.workflow.WorkflowBaseModule  : Starting workflow manager info.magnolia.module.workflow.jbpm.JbpmWorkflowManager...
2015-12-30 00:50:02,485 ...

From beginning of loading a workflow module until next log message from some other module is printed it takes roughly 1.2 seconds, and not all of that time is spent in initialisation of workflow engine. Why should that not be acceptable overhead when changing configuration? It's still significantly less time and annoyance to users than forcing restart of whole server to pickup new engine configuration.

We simply cannot reconfigure the engine once it's initialized (with our current implementation anyways).

Which is why I suggested dropping the instance and obtaining new one.

So I'm saying why stop the engine, unless the system is stopping.

It indeed prevents making workflow unusable and is a good hot fix, but long term, we need to get rid of it and re-instantiate workflow engine anyway so that restart is not required just because someone has changed configuration.

Enviromment as well, timer and scheduler services are deregistered on stop and so on.

This isn't being done by workflow. That's observation kicking in after a configuration change.

Nope, I'm talking about jbpm runtime environment and timer and scheduler services that jbpm uses for scheduled tasks, not about Magnolia scheduler. Check the code of AbstractRuntimeManager.close(boolean) to see for yourself

Comment by Richard Gange [ 30/Dec/15 ]

Ok, I agree that we should allow for the changing of configuration if it's possible. But it does take time to start the JbpmWorkflowManager. All those log statements say is it is starting. When it actually finishes isn't being logged. You can turn on DEBUG to get some exact time measurements.

Generated at Mon Feb 12 10:08:53 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.