Skip to content

[Zen2] Migrate no-master-block integration tests#36502

Merged
DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner:2018-12-11-no-master-block-integration-tests
Dec 12, 2018
Merged

[Zen2] Migrate no-master-block integration tests#36502
DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner:2018-12-11-no-master-block-integration-tests

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

This change follows up on #36478 by migrating the affected integration tests to
use Zen2.

This change follows up on elastic#36478 by migrating the affected integration tests to
use Zen2.
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests v7.0.0 :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Dec 11, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed

Copy link
Copy Markdown
Contributor

@andrershov andrershov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, except Zen setting, which ideally should not be used.

.put(DiscoverySettings.NO_MASTER_BLOCK_SETTING.getKey(), "all")
.build();
.put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), true)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to get rid of this setting, because we're running Zen2 and not Zen? Or should it stay until Zen is completely removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately today InternalTestCluster currently requires that it's explicitly set if autoMinMasterNodes is false, which is why it's present but set ludicrously high. Once we've finished migrating everything to Zen2 it will be easy enough to find all usages like this to remove them.

.put(DiscoverySettings.NO_MASTER_BLOCK_SETTING.getKey(), "write")
.build();
.put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), false)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to get rid of this setting, because we're running Zen2 and not Zen? Or should it stay until Zen is completely removed?

@DaveCTurner
Copy link
Copy Markdown
Member Author

@elasticmachine please run the Gradle build tests 2

.build();
.put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), true)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.put(GatewayService.RECOVER_AFTER_MASTER_NODES_SETTING.getKey(), 3)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why add this setting? This should not be needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It defaults to whatever minimum_master_nodes is. I could just set that to 3.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah that's annoying, and does not make any sense in Zen2. Can you perhaps change GatewayService to not do this if discovery instanceof Coordinator?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I pushed b0e9643.

.build();
.put(AutoCreateIndex.AUTO_CREATE_INDEX_SETTING.getKey(), false)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.put(GatewayService.RECOVER_AFTER_MASTER_NODES_SETTING.getKey(), 3)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here. Why is this needed?

Copy link
Copy Markdown
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DaveCTurner
Copy link
Copy Markdown
Member Author

Apparently-unrelated failures.

@elasticmachine please run the gradle build tests 1

@DaveCTurner DaveCTurner merged commit aa43e0b into elastic:master Dec 12, 2018
@DaveCTurner DaveCTurner deleted the 2018-12-11-no-master-block-integration-tests branch December 12, 2018 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >test Issues or PRs that are addressing/adding tests v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants