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

Add ccr follow info api #37408

Merged
merged 7 commits into from
Jan 18, 2019
Merged

Conversation

martijnvg
Copy link
Member

This api returns all or requested follower indices and per follower index
the provided parameters at put follow / resume follow time and
whether index following is paused or active.

This api is useful for the UI. Figuring out which index is a follower index,
what the active parameters are for a follower index and
whether index follow is active or paused, isn't straightforward and
requires internal ccr knowledge. Having this logic in the UI is the wrong place.

Closes #37127

This api returns all follower indices and per follower index
the provided parameters at put follow / resume follow time and
whether index following is paused or active.

Closes elastic#37127
@martijnvg martijnvg added >feature v7.0.0 :Distributed/CCR Issues around the Cross Cluster State Replication features v6.7.0 labels Jan 14, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

Thanks @martijnvg. This looks good. I left some minor comments.

}
}

public static class FollowParameters implements Writeable {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe do a follow-up to make FollowParameters a separate class then re-use in ShardFollowTask?

`follower_indices`::
(array) an array of follower index statistics

The `indices` array consists of objects containing two fields:
Copy link
Member

Choose a reason for hiding this comment

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

nit: here we are returning more than two fields.

(string) the <modules-remote-clusters,remote cluster>> containing the leader
index

`indices[].leader_cluster`::
Copy link
Member

Choose a reason for hiding this comment

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

nit: leader_cluster -> leader_index

cluster

`indices[].parameters.max_outstanding_read_requests`::
(long) the configured maximum number of outstanding reads requests from the remote
Copy link
Member

Choose a reason for hiding this comment

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

reads requests -> read requests.

(integer) the configured maximum number of outstanding write requests on the follower

`indices[].parameters.max_write_buffer_count`::
(integer) th configurede maximum number of operations that can be queued for writing;
Copy link
Member

Choose a reason for hiding this comment

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

th configurede -> the configured

@dnhatn
Copy link
Member

dnhatn commented Jan 17, 2019

@lcawl Would you please have a look at the doc? Thank you!

@dnhatn dnhatn requested a review from lcawl January 17, 2019 00:13
@martijnvg
Copy link
Member Author

Thanks @dnhatn! I've updated the PR.

The `parameters` contains the following fields:

`indices[].parameters.max_read_request_operation_count`::
(integer) The configured maximum number of operations to pull per read from
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what the purpose of "configured" is here and in other descriptions. Does this mean it only contains that information if you've explicitly set it (i.e. it doesn't appear if you're using default values)?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, if a parameter has not been configured then the default value is returned.
So maybe 'configured' should be removed.

@martijnvg
Copy link
Member Author

@elasticmachine run gradle build tests 1

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM.

@martijnvg martijnvg merged commit 6846666 into elastic:master Jan 18, 2019
martijnvg added a commit that referenced this pull request Jan 18, 2019
This api returns all follower indices and per follower index
the provided parameters at put follow / resume follow time and
whether index following is paused or active.

Closes #37127
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 >feature v6.7.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants