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.

Sign up for GitHub

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

Jump to bottom

[CCR] Rename idle_shard_retry_delay to poll_timout in auto follow patterns #33821

Merged
merged 2 commits into from
Sep 19, 2018
Merged

[CCR] Rename idle_shard_retry_delay to poll_timout in auto follow patterns #33821

merged 2 commits into from
Sep 19, 2018

Conversation

martijnvg
Copy link
Member

No description provided.

@martijnvg martijnvg added review :Distributed/CCR Issues around the Cross Cluster State Replication features labels Sep 18, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

jasontedor
jasontedor approved these changes Sep 18, 2018
View reviewed changes
Copy link
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.

Is the idea here that since we plan to implement long polling for auto-follow patterns, we should name this parameter to poll timeout now (even though it doesn't mean a poll timeout yet)?

If so, I am onboard with this and this LGTM. If not, what is the idea here?

@martijnvg
Copy link
Member Author

Is the idea here that since we plan to implement long polling for auto-follow patterns, we should name this parameter to poll timeout now (even though it doesn't mean a poll timeout yet)?

Sorry I should have described this PR better.

So this parameter, if set, is used to set poll timeout in the follow index request, which then sets the poll timeout of the shard follow task. However the name we used in auto follow patterns was still idle_shard_retry_delay and not poll_timeout like in FollowIndexAction.Request and ShardFollowTask classes, hence this rename.

jasontedor
jasontedor approved these changes Sep 19, 2018
View reviewed changes
Copy link
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.

I see. That’s still fine. LGTM.

@martijnvg martijnvg merged commit d9947c6 into elastic:master Sep 19, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 19, 2018
@jasontedor
Merge branch 'master' into repository-s3-client-override
2239b25 
* master: (46 commits)
  Fixing assertions in integration test (elastic#33833)
  [CCR] Rename idle_shard_retry_delay to poll_timout in auto follow patterns (elastic#33821)
  HLRC: Delete ML calendar (elastic#33775)
  Move DocsStats into Engine (elastic#33835)
  [Docs] Clarify accessing Date methods in painless (elastic#33560)
  add elasticsearch-shard tool (elastic#32281)
  Cut over to unwrap segment reader (elastic#33843)
  SQL: Fix issue with options for QUERY() and MATCH(). (elastic#33828)
  Emphasize that filesystem-level backups don't work (elastic#33102)
  Use the global doc id to generate a random score (elastic#33599)
  Add minimal sanity checks to custom/scripted similarities. (elastic#33564)
  Profiler: Don’t profile NEXTDOC for ConstantScoreQuery. (elastic#33196)
  [CCR] Change FollowIndexAction.Request class to be more user friendly (elastic#33810)
  SQL: day and month name functions tests locale providers enforcement (elastic#33653)
  TESTS: Set SO_LINGER = 0 for MockNioTransport (elastic#32560)
  Test: Relax jarhell gradle test (elastic#33787)
  [CCR] Fail with a descriptive error if leader index does not exist (elastic#33797)
  Add ES version 6.4.2 (elastic#33831)
  MINOR: Remove Some Dead Code in Scripting (elastic#33800)
  Ensure realtime `_get` and `_termvectors` don't run on the network thread (elastic#33814)
  ...
martijnvg added a commit that referenced this pull request Sep 19, 2018
@martijnvg
[CCR] Rename idle_shard_retry_delay to poll_timout in auto follow pat…
e351b45 
…terns (#33821)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasontedor jasontedor jasontedor approved these changes

Assignees
No one assigned
Labels
:Distributed/CCR Issues around the Cross Cluster State Replication features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martijnvg @elasticmachine @jasontedor