Skip to content

[CCR] Change AutofollowCoordinator to use wait_for_metadata_version#36264

Merged
martijnvg merged 4 commits intoelastic:masterfrom
martijnvg:ccr_auto_follow_wait_for_metadata_version
Dec 12, 2018
Merged

[CCR] Change AutofollowCoordinator to use wait_for_metadata_version#36264
martijnvg merged 4 commits intoelastic:masterfrom
martijnvg:ccr_auto_follow_wait_for_metadata_version

Conversation

@martijnvg
Copy link
Copy Markdown
Member

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

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
@martijnvg martijnvg added >enhancement v7.0.0 :Distributed/CCR Issues around the Cross Cluster State Replication features v6.6.0 labels Dec 5, 2018
@martijnvg martijnvg requested a review from jasontedor December 5, 2018 15:17
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed

Copy link
Copy Markdown
Member

@jasontedor jasontedor left a 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.

onFailure,
leaderClusterState -> {
IndexMetaData leaderIndexMetaData = leaderClusterState.getMetaData().index(leaderIndex);
remoteClusterStateRsp -> {
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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.

request,
e -> handler.accept(null, e),
leaderClusterState -> handler.accept(leaderClusterState, null));
remoteClusterStateRsp -> handler.accept(remoteClusterStateRsp, null));
Copy link
Copy Markdown
Member

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.


getLeaderClusterState(remoteCluster, (leaderClusterState, e) -> {
if (leaderClusterState != null) {
getRemoteClusterState(remoteCluster, metadataVersion + 1, (leaderClusterStateRsp, e) -> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remote instead of leader.

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

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.

@martijnvg martijnvg mentioned this pull request Dec 12, 2018
10 tasks
@martijnvg martijnvg merged commit 883940a into elastic:master Dec 12, 2018
martijnvg added a commit that referenced this pull request Dec 12, 2018
…36264)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/CCR Issues around the Cross Cluster State Replication features >enhancement v6.6.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants