> On 20 Jun 2016, at 11:04, Kabir Khan <kabir.khan(a)jboss.com&gt; wrote: > > Jeff and I were chatting about https://issues.jboss.org/browse/WFCORE-1157 last week. There is currently a PR (https://issues.jboss.org/browse/WFCORE-1157?devStatusDetailDialog=pullreq...) 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 executor? > > 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.

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)

> _______________________________________________ > wildfly-dev mailing list > wildfly-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/wildfly-dev _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev

On 20 Jun 2016, at 20:48, Brian Stansberry <brian.stansberry(a)redhat.com&gt; wrote: On 6/20/16 5:04 AM, Kabir Khan wrote: > Jeff and I were chatting about https://issues.jboss.org/browse/WFCORE-1157 last week. There is currently a PR (https://issues.jboss.org/browse/WFCORE-1157?devStatusDetailDialog=pullreq...) 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 executor? > I think it's guaranteed. All notifications go into a single queue and then there is a lock that ensures only 1 thread at a time is processing that queue.

You could skip the locking bit by using a single thread executor, but that means creating a new executor, cleaning it up properly on shutdown, and always having a thread in this pool, mostly doing nothing. > 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. > _______________________________________________ > wildfly-dev mailing list > wildfly-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/wildfly-dev > -- Brian Stansberry Senior Principal Software Engineer JBoss by Red Hat _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev