On 6/20/16 5:08 AM, Kabir Khan wrote:
> On 20 Jun 2016, at 11:04, Kabir Khan <kabir.khan(a)jboss.com> wrote:
> Jeff and I were chatting about https://issues.jboss.org/browse/WFCORE-1157
There is currently a PR
) to allow
listening on the ControlledProcessState state changes. This is done via users registering
NotificationHandlers on the runtime-configuration-state.
> Since the notification handlers are executed asynchronously, there is no guarantee
that e.g. on a stop that the notification handler is triggered for the 'stopping'
and 'stopped' (the PR introduces this latter state) state changes since the server
may be down before this happens. The PR works around this by making the controller execute
the runtime-configuration-state handlers synchronously. However, this then means that the
standard notifications and the runtime-configuration-state notifications end up being in
separate streams, so that the 'stopping' handler may be invoked upon before the
standard/async notification handlers reflecting earlier changes.
> In fact looking at this a bit closer, the NonBlockingNotificationSupport class uses a
thread pool with several threads. This means that for the standard async notifications,
it is very likely that the handler for notification1 gets invoked before
notification2's handler, but is is _not_ guaranteed. If the thread processing
notification1 is paused for whatever reason, notification2 may end up being handled first.
Should we change the executor in NonBlockingNotificationSupport to be a single thread
> Jeff also suggested, perhaps keeping the runtime-configuration-state notifications as
asynchronous, but to add some constructs to make sure that these always get executed
before server shutdown. This would keep the functionality from this PR where the
notifications are always invoked, and also make sure that the order is preserved.
That sounds better than having some notifications mysteriously be sync
and others async. But, as you say...
There may be problems with this though. If e.g. one of the lifecycle
handlers needs access to services etc. they may have been stopped before the
stopped/stopping notifications are invoked. But that could perhaps already happen with the
current sync approach (I need to look and refamiliariase myself)
I've copied Ramesh Reddy, as I think he may be one of the users of the
existing sync notifications via ControlledProcessStateService. At least
I believe I pointed him to that in the past. IIRC he would have wanted
to get the notification before the services started shutting down, in
order to use that knowledge in detecting whether services were stopping
due to shutdown vs mgmt op initiated removal (e.g. undeploy.) Ramesh, is
my memory correct?
The existing ControlledProcessStateService stuff will result in a
stopping notification being received before services shut down. The
process state is changed and notifications emitted before the thread
moves on to doing things that affect MSC.
I think we need a bit of a rethink of this whole thing. We have some
seemingly conflicting use cases, e.g. wanting sync notifications of
lifecycle (or even startup completion if the user wants to racelessly
change something before requests start being handled), and also wanting
to make changes (e.g. with a ModelControllerClient) when those events
happen, which sometimes must be done async to avoid deadlock.
One thought I've had around this is whether a cmd line flag to start up
suspended would help with some use cases. (I think that's a useful thing
anyway for other uses.) With that a user can get a notification of
"running" but the server is still suspended. So they can tweak whatever
and then tip the server out of suspended state.
> wildfly-dev mailing list
wildfly-dev mailing list
Senior Principal Software Engineer
JBoss by Red Hat