Test behaviour depends on JDK set/map iteration ordering which varies independently of the test seed #94946
Labels
>bug
:Core/Infra/Core
Core issues without another label
Team:Core/Infra
Meta label for core/infra team
Comments
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this issue
Mar 31, 2023
Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `DiscoveryNodes#mastersFirstStream` yield the nodes in a deterministic order, which addresses one such area of unreproducibility. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates elastic#94946
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this issue
Mar 31, 2023
Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `NodeConnectionsService#connectToNodes` iterate through the nodes using `DiscoveryNodes#mastersFirstStream`, which is made deterministic by elastic#94948. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates elastic#94946
elasticsearchmachine
pushed a commit
that referenced
this issue
Mar 31, 2023
Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `DiscoveryNodes#mastersFirstStream` yield the nodes in a deterministic order, which addresses one such area of unreproducibility. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates #94946
elasticsearchmachine
pushed a commit
that referenced
this issue
Mar 31, 2023
Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `NodeConnectionsService#connectToNodes` iterate through the nodes using `DiscoveryNodes#mastersFirstStream`, which is made deterministic by #94948. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates #94946
DaveCTurner
changed the title
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this issue
Apr 5, 2023
A handful of small changes to make the logging output of `CoordinatorTests` even more deterministic, for easier diffing. Relates elastic#94946
DaveCTurner
added a commit
that referenced
this issue
Apr 5, 2023
A handful of small changes to make the logging output of `CoordinatorTests` even more deterministic, for easier diffing. Relates #94946
rjernst
pushed a commit
to rjernst/elasticsearch
that referenced
this issue
Apr 6, 2023
Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `DiscoveryNodes#mastersFirstStream` yield the nodes in a deterministic order, which addresses one such area of unreproducibility. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates elastic#94946
rjernst
pushed a commit
to rjernst/elasticsearch
that referenced
this issue
Apr 6, 2023
…4949) Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `NodeConnectionsService#connectToNodes` iterate through the nodes using `DiscoveryNodes#mastersFirstStream`, which is made deterministic by elastic#94948. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates elastic#94946
rjernst
pushed a commit
to rjernst/elasticsearch
that referenced
this issue
Apr 6, 2023
A handful of small changes to make the logging output of `CoordinatorTests` even more deterministic, for easier diffing. Relates elastic#94946
saarikabhasi
pushed a commit
to saarikabhasi/elasticsearch
that referenced
this issue
Apr 10, 2023
Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `DiscoveryNodes#mastersFirstStream` yield the nodes in a deterministic order, which addresses one such area of unreproducibility. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates elastic#94946
saarikabhasi
pushed a commit
to saarikabhasi/elasticsearch
that referenced
this issue
Apr 10, 2023
…4949) Today the `CoordinatorTests` test suite is not totally deterministic because its behaviour depends on the iteration order of the JDK's unordered collections which are not under the control of our test seed. This commit makes `NodeConnectionsService#connectToNodes` iterate through the nodes using `DiscoveryNodes#mastersFirstStream`, which is made deterministic by elastic#94948. It's an ugly hack to do some extra work in production just for the sake of tests, but we're only sorting at most a few hundred elements here so it's not a huge deal. Relates elastic#94946
saarikabhasi
pushed a commit
to saarikabhasi/elasticsearch
that referenced
this issue
Apr 10, 2023
A handful of small changes to make the logging output of `CoordinatorTests` even more deterministic, for easier diffing. Relates elastic#94946
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Various of our test suites (most notably
CoordinatorTests) have behaviour which changes depending on the order in which we iterate through a set or a map. Since we moved from HPPC's collections to JDK's immutableSetandMapimplementations we have lost the ability to make this iteration order depend on the test seed. That means that if one of these tests fails we cannot reliably reproduce the failure (e.g. after adding more logging).Note that it is not that we want to fix the iteration order in any way: all orders are valid test cases. It's just that we want to be able to iterate the same collection in the same order in a different invocation of the JVM so that we can investigate the behaviour that led to a test failure.
See also https://mail.openjdk.org/pipermail/core-libs-dev/2023-March/102865.html.
(I'm opening this issue mainly so I have somewhere to link some related work)
94946in their vicinity):org.elasticsearch.cluster.node.DiscoveryNodes#MASTERS_FIRST_COMPARATORorg.elasticsearch.cluster.NodeConnectionsService#connectToNodesThe text was updated successfully, but these errors were encountered: