Skip to content

Commit

Permalink
[test] Remove synchronization from InternalTestCluster#getInstance (#…
Browse files Browse the repository at this point in the history
…117780)

The map of nodes is volatile and immutable and can be ready without
synchronization. Getting a class's instance from the node's injector
is also thread safe.

Doing so prevents deadlocks if we restart the node and have a disruption
scheme that internally calls `getInstance` from another thread.

```
  2> "elasticsearch[StatelessClusterIntegrityStressIT][server][T#1]" ID=3490 BLOCKED on org.elasticsearch.test.InternalTestCluster@18a6d098 owned by "elasticsearch[StatelessClusterIntegrityStressIT][server][T#2]" ID=3492
  2> 	at app//org.elasticsearch.test.InternalTestCluster.getInstance(InternalTestCluster.java:1653)
  2> 	- blocked on org.elasticsearch.test.InternalTestCluster@18a6d098
  2> 	at app//org.elasticsearch.test.InternalTestCluster.getInstance(InternalTestCluster.java:1620)
  2> 	at app//org.elasticsearch.test.disruption.NetworkDisruption.transport(NetworkDisruption.java:172)
  2> 	at app//org.elasticsearch.test.disruption.NetworkDisruption.applyToNodes(NetworkDisruption.java:157)
  2> 	at app//org.elasticsearch.test.disruption.Net  2> workDisruption.startDisrupting(NetworkDisruption.java:133)

 2> "elasticsearch[StatelessClusterIntegrityStressIT][server][T#2]" ID=3492 BLOCKED on org.elasticsearch.test.disruption.NetworkDisruption@60fd3a1e owned by "elasticsearch[StatelessClusterIntegrityStressIT][server][T#1]" ID=3490
   2> 	at app//org.elasticsearch.test.disruption.NetworkDisruption.applyToNode(NetworkDisruption.java:116)
   2> 	- blocked on org.elasticsearch.test.disruption.NetworkDisruption@60fd3a1e
   2> 	at app//org.elasticsearch.test.InternalTestCluster.applyDisruptionSchemeToNode(InternalTestCluster.java:2307)
   2> 	at app//org.elasticsearch.test.InternalTestCluster.publishNode(InternalTestCluster.java:2258)
   2> 	- locked org.elasticsearch.test.InternalTestCluster@18a6d098
   2> 	at app//org.elasticsearch.test.InternalTestCluster.restartNode(InternalTestCluster.java:1901)
   2> 	at app//org.elasticsearch.test.InternalTestCluster.restartNode(InternalTestCluster.java:1863)
   2> 	- locked org.elasticsearch.test.InternalTestCluster@18a6d098
 ```
  • Loading branch information
arteam authored Dec 3, 2024
1 parent ae1b132 commit 5ed106a
Showing 1 changed file with 1 addition and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,7 @@ public <T> T getAnyMasterNodeInstance(Class<T> clazz) {
return getInstance(clazz, MASTER_NODE_PREDICATE);
}

private synchronized <T> T getInstance(Class<T> clazz, Predicate<NodeAndClient> predicate) {
private <T> T getInstance(Class<T> clazz, Predicate<NodeAndClient> predicate) {
NodeAndClient randomNodeAndClient = getRandomNodeAndClient(predicate);
if (randomNodeAndClient == null) {
throw new AssertionError("no node matches [" + predicate + "]");
Expand Down

0 comments on commit 5ed106a

Please sign in to comment.