[Zen2] Update default for USE_ZEN2 to true #35998
Conversation
Today the default for USE_ZEN2 is false and it is overridden in many places. By defaulting it to true we can be sure that the only places in which Zen2 does not work are those in which it is explicitly set to false.
DaveCTurner
added
>enhancement
v7.0.0
:Distributed Coordination/Cluster Coordination
|
Pinging @elastic/es-distributed |
DaveCTurner
changed the title
| @@ -66,7 +66,6 @@ public void testNodeVersionIsUpdated() throws IOException, NodeValidationExcepti | |||
| .put("transport.type", getTestTransportType()) | |||
| .put(Node.NODE_DATA_SETTING.getKey(), false) | |||
| .put("cluster.name", "foobar") | |||
| .put(TestZenDiscovery.USE_ZEN2.getKey(), true) | |||
There was a problem hiding this comment.
This (and some of the other occurrences removed in this PR) should be set to Zen1 or Zen2 depending on whether the surrounding test uses the corresponding disco type. If we want to randomize the tests to run with Zen1 / Zen2 for a while, it's probably better to add a field useZen2 to ESTestCase (similar to useNio) that will for now be set to Zen2, but could also be set to randomBoolean() for a while. This line here should then be changed to .put(TestZenDiscovery.USE_ZEN2.getKey(), getUseZen2())
There was a problem hiding this comment.
Ok, I pushed 7bdc19a.
On the one hand this feels like premature generalisation. On the other hand, simply removing the places where today we use USE_ZEN2 makes it harder to find them again later, and more likely that we miss some of them when trying to run mixed tests.
test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
Show resolved
Hide resolved
… the configuration" This reverts commit 48eb72c - committed on the wrong branch.
| @@ -33,7 +33,7 @@ | |||
| public class PersistedStateIT extends ESIntegTestCase { | |||
|
|
|||
| private static Settings SETTINGS = Settings.builder() | |||
| .put(TestZenDiscovery.USE_ZEN2.getKey(), true) | |||
| .put(TestZenDiscovery.USE_ZEN2.getKey(), getUseZen2()) | |||
There was a problem hiding this comment.
this test was meant to specifically test Zen2
so maybe just set it to true here.
Note that this test can probably go away once state recovery is implemented.
|
Last change was trivial and passed precommit, so merging. |
Today the default for USE_ZEN2 is false and it is overridden in many places. By
defaulting it to true we can be sure that the only places in which Zen2 does
not work are those in which it is explicitly set to false.