-
Notifications
You must be signed in to change notification settings - Fork 25k
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
[CCR] Change AutofollowCoordinator to use wait_for_metadata_version #36264
[CCR] Change AutofollowCoordinator to use wait_for_metadata_version #36264
Conversation
Changed AutofollowCoordinator makes use of the wait_for_metadata_version feature in cluster state API and removed hard coded poll interval. Originates from elastic#35895 Relates to elastic#33007
Pinging @elastic/es-distributed |
…r_metadata_version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Sorry for some nits.
@@ -121,8 +121,9 @@ public void checkRemoteClusterLicenseAndFetchLeaderIndexMetadataAndHistoryUUIDs( | |||
client.getRemoteClusterClient(clusterAlias), | |||
request, | |||
onFailure, | |||
leaderClusterState -> { | |||
IndexMetaData leaderIndexMetaData = leaderClusterState.getMetaData().index(leaderIndex); | |||
remoteClusterStateRsp -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess “Rsp
is Response
? Sorry to be a nit, can we spell it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was my attempt to keep line length at bay. I will change it.
// TODO: set non-compliant status on auto-follow coordination that can be viewed via a stats API | ||
ccrLicenseChecker.checkRemoteClusterLicenseAndFetchClusterState( | ||
client, | ||
remoteCluster, | ||
request, | ||
e -> handler.accept(null, e), | ||
leaderClusterState -> handler.accept(leaderClusterState, null)); | ||
remoteClusterStateRsp -> handler.accept(remoteClusterStateRsp, null)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here about spelling out to Response
.
@@ -272,10 +268,15 @@ void autoFollowIndices() { | |||
this.autoFollowPatternsCountDown = new CountDown(patterns.size()); | |||
this.autoFollowResults = new AtomicArray<>(patterns.size()); | |||
|
|||
getLeaderClusterState(remoteCluster, (leaderClusterState, e) -> { | |||
if (leaderClusterState != null) { | |||
getRemoteClusterState(remoteCluster, metadataVersion + 1, (leaderClusterStateRsp, e) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remote
instead of leader
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And of course same comment to spell out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#36408 already changes this method, so when I merge in master after that pr is merged it should be good.
…r_metadata_version
Changed AutofollowCoordinator makes use of the wait_for_metadata_version
feature in cluster state API and removed hard coded poll interval.
Originates from #35895
Relates to #33007