[CCR] Clean followed leader index UUIDs in auto follow metadata#36408
Conversation
The auto follow coordinator keeps track of the UUIDs of indices that it has followed. The index UUID strings need to be cleaned up in the case that these indices are removed in the remote cluster. Relates to elastic#33007
|
Pinging @elastic/es-distributed |
jasontedor
left a comment
There was a problem hiding this comment.
Looks good. I left a few comments. They are pretty minor.
| } | ||
| i++; | ||
| } | ||
| getLeaderClusterState(remoteCluster, (remoteClusterState, remoteError) -> { |
There was a problem hiding this comment.
I guess this should be renamed to getRemoteClusterState now.
| for (String autoFollowPatternName : patterns) { | ||
| results.add(new AutoFollowResult(autoFollowPatternName, e)); | ||
| for (int i = 0; i < patterns.size(); i++) { | ||
| String autoFollowPatternName = patterns.get(i); |
There was a problem hiding this comment.
Maybe an assertion that remoteError is not null?
| } | ||
|
|
||
| void cleanFollowedLeaderIndices(final ClusterState remoteClusterState, | ||
| final List<String> patterns) { |
There was a problem hiding this comment.
These parameters should fit on online.
| }; | ||
| } | ||
|
|
||
| void cleanFollowedLeaderIndices(final ClusterState remoteClusterState, |
There was a problem hiding this comment.
Continuing with the theme, let us leave "leader" out of this method name and instead prefer "remote".
| }); | ||
| } | ||
|
|
||
| static Function<ClusterState, ClusterState> cleanFollowedLeaderIndices(final MetaData remoteMetadata, |
| } | ||
|
|
||
| static Function<ClusterState, ClusterState> cleanFollowedLeaderIndices(final MetaData remoteMetadata, | ||
| final List<String> autoFollowPatternNames) { |
There was a problem hiding this comment.
How about
diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java
index b3e437bc381..8fe89b3ae35 100644
--- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java
+++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java
@@ -476,8 +476,8 @@ public class AutoFollowCoordinator implements ClusterStateListener {
});
}
- static Function<ClusterState, ClusterState> cleanFollowedLeaderIndices(final MetaData remoteMetadata,
- final List<String> autoFollowPatternNames) {
+ static Function<ClusterState, ClusterState> cleanFollowedLeaderIndices(
+ final MetaData remoteMetadata, final List<String> autoFollowPatternNames) {
return currentState -> {
AutoFollowMetadata currentAutoFollowMetadata = currentState.metaData().custom(AutoFollowMetadata.TYPE);
Map<String, List<String>> newFollowedIndexUUIDS = new HashMap<>(currentAutoFollowMetadata.getFollowedLeaderIndexUUIDs());| final List<String> autoFollowPatternNames) { | ||
| return currentState -> { | ||
| AutoFollowMetadata currentAutoFollowMetadata = currentState.metaData().custom(AutoFollowMetadata.TYPE); | ||
| Map<String, List<String>> newFollowedIndexUUIDS = new HashMap<>(currentAutoFollowMetadata.getFollowedLeaderIndexUUIDs()); |
There was a problem hiding this comment.
I think this needs a rename. I got confused by it a few times later on in the code. Perhaps autoFollowPatternNameToFollowedIndexUUIDs?
| continue; | ||
| } | ||
|
|
||
| List<String> followedLeaderIndices = new ArrayList<>(newFollowedIndexUUIDS.get(autoFollowPatternName)); |
There was a problem hiding this comment.
followedLeaderIndices -> followedIndexUUIDs?
…_followed_indices
…es in cluster, due that the fact that no good diffs could be determined of AutoFollowMetadata.
|
@jasontedor I've updated the PR. I noticed that new test ( |
…_followed_indices
jasontedor
left a comment
There was a problem hiding this comment.
The production code LGTM. Sorry for one more naming nit though. No need for another round!
| if (leaderIndicesToFollow.isEmpty()) { | ||
| finalise(slot, new AutoFollowResult(autoFollowPatternName)); | ||
| } else { | ||
| List<Tuple<String, AutoFollowPattern>> patternsForTheSameLeaderCluster = autoFollowMetadata.getPatterns() |
There was a problem hiding this comment.
LeaderCluster -> RemoteCluster?
There was a problem hiding this comment.
I should have done find and replace case insensitive... it is more difficult to get rid of leaderCluster and variants than I thought it would be.
The auto follow coordinator keeps track of the UUIDs of indices that it has followed. The index UUID strings need to be cleaned up in the case that these indices are removed in the remote cluster. Relates to #33007
The auto follow coordinator keeps track of the UUIDs of indices that it has followed. The index UUID strings need to be cleaned up in the case that these indices are removed in the remote cluster.
Relates to #33007