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

Stop auto-followers when resetting metadata #81290

Merged
merged 5 commits into from
Dec 7, 2021

Conversation

idegtiarenko
Copy link
Contributor

This change ensures auto-followers are stopped if auto-follow metadata is reverted back to null after some non-empty state.

Fixes #81242 (though will not happen until #81247 is fixed)

@idegtiarenko idegtiarenko added >bug :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features v8.0.0 Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. auto-backport-and-merge v8.1.0 labels Dec 3, 2021
@elasticmachine
Copy link
Collaborator

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

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.

LGTM.

return;
}
final AutoFollowMetadata autoFollowMetadata = followerClusterState.getMetadata()
.custom(AutoFollowMetadata.TYPE, AutoFollowMetadata.EMPTY);

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we bail out early here if autoFollowMetadata.getPatterns().isEmpty() && this.autoFollowers.isEmpty()? Since this runs on every cluster change it would be nice to have a bail out.


// then auto-followers are removed
assertThat(newAutoFollowers.entrySet(), empty());
// and remotes are stopped
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// and remotes are stopped
// and auto-followers are stopped

ClusterState clusterState = ClusterState.builder(new ClusterName("remote")).build();
autoFollowCoordinator.updateAutoFollowers(clusterState);
var autoFollowCoordinator = createAutoFollowCoordinator();
autoFollowCoordinator.updateAutoFollowers(createClusterStateWith(null));
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps randomly choose null or Map.of()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is already another test case that checks transition from non empty to empty map.

@idegtiarenko
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/bwc

@idegtiarenko idegtiarenko merged commit bdc1b73 into elastic:master Dec 7, 2021
@idegtiarenko idegtiarenko deleted the 81242 branch December 7, 2021 13:33
idegtiarenko added a commit to idegtiarenko/elasticsearch that referenced this pull request Dec 7, 2021
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.0
7.16 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 81290

idegtiarenko added a commit to idegtiarenko/elasticsearch that referenced this pull request Dec 7, 2021
elasticsearchmachine pushed a commit that referenced this pull request Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.16.1 v8.0.0-rc1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-follow pattern and remote might not be stopped when restoring from old snapshot with global state
5 participants