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

Adds discovery_max_stale #4004

Merged
merged 25 commits into from
Mar 30, 2018
Merged

Adds discovery_max_stale #4004

merged 25 commits into from
Mar 30, 2018

Conversation

preetapan
Copy link
Contributor

Based on PR #3920

pierresouchay and others added 24 commits February 26, 2018 17:24
Following conversation #3911

This patch allow to change the default behavior of Consul regarding
consistency
…lelism to avoid timing issues on their contended CPUs
This avoids a conflict with #datacenter later on the page. We're mixing
histroic manually specified anchors with generated anchors (via
redcarpet / middleman-hashicorp) so we have to manually override the
automatic generation here.

I was tempted to rewrite the old manual anchors to use the automatic
generation, but there is no way to maintain backwards compatibility,
so will leave that for a time when it is appropriate for us to break
links (or redirect them, etc).

Fixes #3916
Minor fixes (typos in comments)
Removed fmt.Printf debug code
Added documentation to new option
@@ -461,6 +461,14 @@ type RuntimeConfig struct {
// flag: -datacenter string
Datacenter string

// Defines the maximum stale value for discovery path. Defauls to "0s".
// Discovery paths are /v1/heath/ paths
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

@mitchellh mitchellh deleted the f-discovery-max-stale branch September 6, 2018 19:00
@sandstrom
Copy link
Contributor

sandstrom commented Sep 17, 2018

@preetapan @pierresouchay Very useful! 🏅

Have you considered switching the default to 87600h to match the current default for max_stale?
(I understand that it's a breaking change and would need to wait until Consul 2.x)

In my view it makes sense to mirror stale-logic for DNS and HTTP discovery requests. However, there may be good reasons to not do that, which I'm unaware of.

Curious to hear your thoughts!

@pierresouchay
Copy link
Contributor

@sandstrom Agree with you, you can follow conversation here: #3911 and #3920

On our side, we are using it with very long values in a similar way to DNS

As you suggested, this is a breaking change, so not for a 1.x version

@sandstrom
Copy link
Contributor

@pierresouchay Thanks, I've read those too! Let's see what @preetapan says about flipping the default in the next major.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants