Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SearchableSnapshotsLicenseIntegTests.testShardAllocationOnInvalidLicense #72528

Merged
merged 1 commit into from
May 3, 2021

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Apr 30, 2021

This test fails sometimes on CI (see #72329) when recreating the license. It's not clear to me why that happens but I suspect batched cluster state updates, so this pull request adds some waiting points in the test.

Closes #72329

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.14.0 labels Apr 30, 2021
@tlrx tlrx requested a review from original-brownbear April 30, 2021 08:55
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Apr 30, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -89,6 +89,9 @@ public void createAndMountSearchableSnapshot() throws Exception {

assertAcked(client().execute(DeleteLicenseAction.INSTANCE, new DeleteLicenseRequest()).get());
assertAcked(client().execute(PostStartBasicAction.INSTANCE, new PostStartBasicRequest()).get());

ensureClusterSizeConsistency();
Copy link
Member

Choose a reason for hiding this comment

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

NIT: I think we can do it in one line with ensureStableCluster(cluster().size())? That seems like it's all we need here? Looks like this is a retry on the master node request I guess? (though there's no master fail-over, but maybe a connection got dropped for some reason?) ... That's my best guess :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I merged as is, I wanted to ensure no pending tasks on all nodes with the same cluster state everywhere.

And yes, retries might explain this. We'll see if that happens again

@tlrx tlrx merged commit a22a0a8 into elastic:master May 3, 2021
@tlrx tlrx deleted the fix-72329 branch May 3, 2021 08:12
@tlrx
Copy link
Member Author

tlrx commented May 3, 2021

Thanks Armin

tlrx added a commit to tlrx/elasticsearch that referenced this pull request May 3, 2021
…License (elastic#72528)

This test fails sometimes on CI (see elastic#72329) when recreating the 
license. It's not clear to me why that happens but I suspect batched 
cluster state updates, so this pull request adds some waiting points 
in the test.

Closes elastic#72329
tlrx added a commit that referenced this pull request May 3, 2021
…License (#72528) (#72611)

This test fails sometimes on CI (see #72329) when recreating the 
license. It's not clear to me why that happens but I suspect batched 
cluster state updates, so this pull request adds some waiting points 
in the test.

Backport of #72528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. >test Issues or PRs that are addressing/adding tests v7.14.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SearchableSnapshotsLicenseIntegTests.testShardAllocationOnInvalidLicense fails when recreating its license
4 participants