wildfly-core icon indicating copy to clipboard operation
wildfly-core copied to clipboard

[WFCORE-6662] Remove deprecated usage of ServiceController.getValue() by capturing ModelControllerClientFactory ...

Open yersan opened this issue 1 year ago • 7 comments

...via ServiceActivator starting the Bootable JAR and Embedded server

This PR follows up on #5824, also built on top of that one.

Jira issue: https://issues.redhat.com/browse/WFCORE-6662

yersan avatar Jan 15 '24 13:01 yersan

Core -> Full Integration Build 13403 outcome was FAILURE using a merge of ebe0d121e1901920be522173ee051eacd3603dfe Summary: Tests failed: 1 (1 new), passed: 7685, ignored: 124 Build time: 03:44:32

Failed tests

org.jboss.as.test.clustering.cluster.ejb.timer.DistributedTimerServiceTestCase.test: java.lang.AssertionError: class org.jboss.as.test.clustering.cluster.ejb.timer.beans.CalendarPersistentTimerBean={node-1=[]}. Actual: 0
	at org.jboss.as.test.clustering.cluster.ejb.timer.AbstractTimerServiceTestCase.test(AbstractTimerServiceTestCase.java:302)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor35.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor34.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor33.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor5.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor3.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor15.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor14.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor5.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at jdk.internal.reflect.GeneratedMethodAccessor3.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
------- Stdout: -------
node-2  [0m15:01:08,201 INFO  [org.jboss.as.repository] (management-handler-thread - 1) WFLYDR0001: Content added at location /opt/buildAgent/work/e8e0dd9c7c4ba60/full/testsuite/integration/clustering/target/wildfly-2/standalone/data/content/91/bcbbcb9e7d8d7ffececc01debb8531ac30fd9e/content
 [0mnode-2  [0m15:01:08,205 INFO  [org.jboss.as.server.deployment] (MSC service thread 1-5) WFLYSRV0027: Starting deployment of "DistributedTimerServiceTestCase.war" (runtime-name: "DistributedTimerServiceTestCase.war")
 [0mnode-2  [0m15:01:08,547 INFO  [org.jboss.weld.deployer] (MSC service thread 1-2) WFLYWELD0003: Processing weld deployment DistributedTimerServiceTestCase.war
 [0mnode-2  [0m15:01:08,558 INFO  [org.jboss.as.ejb3.deployment] (MSC service thread 1-2) WFLYEJB0473: JNDI bindings for session bean named 'AutoTransientTimerBean' in deployment unit 'deployment "DistributedTimerServiceTestCase.war"' are as follows:

	java:global/DistributedTimerServiceTestCase/AutoTransientTimerBean!org.jboss.as.test.clustering.cluster.ejb.timer.beans.TimerBean
	java:app/DistributedTimerServiceTestCase/AutoTransientTimerBean!org.jboss.as.test.clustering.cluster.ejb.timer.beans.TimerBean
	java:module/AutoTransientTimerBean!org.jboss.as.test.clustering.cluster.ejb.timer.beans.TimerBean
	java:global/DistributedTimerServiceTestCase/AutoTransientTimerBean
	java:app/DistributedTimerServiceTestCase/AutoTransientTimerBean
	java:module/AutoTransientTimerBean

 [0mnode-2  [0m15:01:08,559 INFO  [org.jboss.as.ejb3.deployment] (MSC service thread 1-2) WFLYEJB0473: JNDI bindings for session bean named 'SingleActionPersistentTimerBean' in deployment unit 'deployment "DistributedTimerServiceTestCase.war"' are as follows:

	java:global/DistributedTimerServiceTestCase/SingleActionPersistentTimerBean!org.jboss.as.test.clustering.cluster.ejb.timer.beans.ManualTimerBean
	java:app/DistributedTimerServiceTestCase/SingleActionPersistentTimerBean!org.jboss.as.test.clustering.cluster.ejb.timer.beans.ManualTimerBean
	java:module/SingleActionPersistentTimerBean!org.jboss.as.test.clustering.cluster.ejb.timer.beans.ManualTimerBean
	java:global/DistributedTimerServiceTestCase/SingleActionPersistentTimerBean
	java:app/DistributedTimerServiceTestCase/SingleActionPersistentTimerBean
	java:module/SingleActionPersistentTimerBean

 [0mnode-2  [0m15:01:08,559 INFO  [org.jboss.as.ejb3.deployment] (MSC service thread 1-2) WFLYEJB0473: JNDI bindings for session bean named 'AutoPersistentTimerBean' in deployment unit 'deployment "DistributedTimerServiceTestCase.war"' are as follows:



wildfly-ci avatar Jan 15 '24 17:01 wildfly-ci

@yersan Cool! I filed https://issues.redhat.com/browse/WFCORE-6659 for this and gave it a shot in https://github.com/wildfly/wildfly-core/commit/303beed34ce451e7f75a95ebb88d95603d6a8e8b, but it failed CI so I shelved it. It looks like yours here works. :)

I'll review it after I get unburied a bit from PTO.

bstansberry avatar Jan 15 '24 20:01 bstansberry

@bstansberry oohh, I missed the https://issues.redhat.com/browse/WFCORE-6659 notification after my PTOs :(

It looks like there is still some stuff to do with this PR, after taking a quick look at your changes, this one is pretty similar, although this one uses the null value on the notifier to establish when the dependent simple services that provide the (ProcessStateNotifier or ModelControllerClientFactory) are up. In your PR, you are using a Future.

The ProcessStateNotifier for the Server and EmbeddedServer is not accessed via the notifierRef when the service that provides its value has not been started yet, but the ModelControllerClientFactory could be accessed under this circumstance on the establishModelControllerClient private method.

This PR also assumes that there are no service reloads in between once the Server and EmbeddedServer have started. That could be an issue, but I think only for the modelControllerClient.

This PR has the issue that it is not correctly cleaning up the clientFactorySvcCaptureRef / notifierRef when the dependent service is down. For the notifierRef I think it is optional, but that could be an issue for clientFactorySvcCaptureRef.

I think I can fix it by using the intermediate service that provides the value of those AtomicReference notifiers, setting them to null if the dependent service goes down. That would require keeping this intermediate service up until the dependent ones are still up.

yersan avatar Jan 16 '24 11:01 yersan

Partially resolved, it is still racy between the stop method and getting the values from the notifiers (it was also racy before with the original code where createSuperUserClient could be called when the service was down, I guess there are low probabilities, but that's racy). This version also does not remove the intermediate services as soon as the value from the dependents is received. So, it still needs some work.

yersan avatar Jan 16 '24 12:01 yersan

The ProcessStateNotifier for the Server and EmbeddedServer is not accessed via the notifierRef when the service that provides its value has not been started yet, but the ModelControllerClientFactory could be accessed under this circumstance on the establishModelControllerClient private method.

I initially did mine without the Future and got test failures because the getModelControllerClient() call in EmbedServerHandler.doHandle would return null. That was because the establishModelControllerClient call in StandaloneServerImpl.start would not create a client, because the clientFactorySvcCaptureRef.get() call would return null. That's what led to my comment at https://issues.redhat.com/browse/WFCORE-6659?focusedId=23738156&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-23738156

bstansberry avatar Jan 16 '24 23:01 bstansberry

Partially resolved, it is still racy between the stop method and getting the values from the notifiers (it was also racy before with the original code where createSuperUserClient could be called when the service was down, I guess there are low probabilities, but that's racy). This version also does not remove the intermediate services as soon as the value from the dependents is received. So, it still needs some work.

Yes, the createSuperUserClient call should only happen if the clientFactorySvcCaptureRef.get() doesn't return null.

Note that it's ok for ModelControllerClient use to fail in the middle of a reload. If you create a client to a remote server and someone (including yourself) reloads the server, that client isn't expected to work through that. The CLI uses a client that is designed to handle that. The client we return from getModelControllerClient does as well.

bstansberry avatar Jan 16 '24 23:01 bstansberry

There has been no activity on this PR for 45 days. It will be auto-closed after 90 days.

github-actions[bot] avatar Mar 02 '24 00:03 github-actions[bot]

There has been no activity on this PR for 90 days and it has been closed automatically.

github-actions[bot] avatar May 31 '24 00:05 github-actions[bot]