Skip to content

Fix TransportDesiredNodesActionsIT batch tests #83406

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

Merged
merged 6 commits into from
Feb 8, 2022

Conversation

fcofdez
Copy link
Contributor

@fcofdez fcofdez commented Feb 2, 2022

Select the current master to submit the blocking cluster state
update task, otherwise the task never gets executed.

Closes #83386

Select the current master to submit the blocking cluster state
update task.

Closes elastic#83386
@fcofdez fcofdez added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.1.0 labels Feb 2, 2022
@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor

@DaveCTurner DaveCTurner 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, I left another suggestion too.

@@ -372,7 +370,7 @@ private UpdateDesiredNodesResponse updateDesiredNodes(DesiredNodes desiredNodes)
private Runnable blockClusterStateUpdateThread() throws InterruptedException {
final CountDownLatch unblockClusterStateUpdateTask = new CountDownLatch(1);
final CountDownLatch blockingClusterStateUpdateTaskExecuting = new CountDownLatch(1);
final ClusterService clusterService = internalCluster().getMasterNodeInstance(ClusterService.class);
final ClusterService clusterService = internalCluster().getCurrentMasterNodeInstance(ClusterService.class);
Copy link
Contributor

@DaveCTurner DaveCTurner Feb 2, 2022

Choose a reason for hiding this comment

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

This API is so trappy. I opened #83407.

@@ -383,7 +381,8 @@ public ClusterState execute(ClusterState currentState) throws Exception {

@Override
public void onFailure(Exception e) {

blockingClusterStateUpdateTaskExecuting.countDown();
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend also bounding the wait time (assertTrue(blockingClusterStateUpdateTaskExecuting.await(10, TimeUnit.SECONDS)); and similarly for unblockClusterStateUpdateTask).

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM2

@mark-vieira mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022
@fcofdez fcofdez added v8.1.0 auto-backport Automatically create backport pull requests when merged labels Feb 8, 2022
@fcofdez fcofdez merged commit 1b91bf6 into elastic:master Feb 8, 2022
fcofdez added a commit to fcofdez/elasticsearch that referenced this pull request Feb 8, 2022
Select the current master to submit the blocking cluster state
update task, otherwise the task never gets executed.

Closes elastic#83386
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.1

fcofdez added a commit that referenced this pull request Feb 8, 2022
Select the current master to submit the blocking cluster state
update task, otherwise the task never gets executed.

Closes #83386
Backport of #83406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. >test Issues or PRs that are addressing/adding tests v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] TransportDesiredNodesActionsIT testDeleteDesiredNodesTasksAreBatchedCorrectly failing
6 participants