Added a way to override default consistency level per agent #3920

pierresouchay · 2018-02-26T16:31:49Z

Following conversation #3911

This patch allow to change the default behavior of Consul regarding
consistency

It still requires discussion probably and futher test, just a start for discussion

Following conversation hashicorp#3911 This patch allow to change the default behavior of Consul regarding consistency

…lelism to avoid timing issues on their contended CPUs

… to disk

https://docs.travis-ci.com/user/reference/overview/#Virtualisation-Environment-vs-Operating-System

…delay gathering packages until the makefile

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 hashicorp#3916

…tency

pierresouchay · 2018-03-13T21:27:02Z

Hello @banks,

What do you think about this kind of implementation, would it be mergeable on long term?

Regards

banks · 2018-03-14T12:47:55Z

I'll discuss again with the team on our Friday call and see if there is consensus on this. Thanks for all you effort chasing down these optimisations.

preetapan

@pierresouchay I just got a chance to review this and added some more comments. We are ready to take this after that though, so this will probably get merged this week after you address my latest feedback.

preetapan · 2018-03-27T16:09:14Z

agent/config/runtime.go

@@ -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/catalog/* and /v1/heath/* paths


preetapan · 2018-03-27T16:11:22Z

agent/http.go

+		defaults = false
+	}
+	if maxStale := query.Get("max_stale"); maxStale != "" {
+		fmt.Printf("**** max_stale=%s\n", maxStale)


Remove this

Ooops... sorry, fixed

preetapan · 2018-03-27T16:12:58Z

agent/http_test.go

@@ -632,13 +634,70 @@ func TestParseConsistency(t *testing.T) {
 	}
 }

+// ensureConsistency check if consistency modes are correclty applied


typo in correctly

preetapan · 2018-03-27T16:21:38Z

agent/http.go

+	if defaults {
+		path := req.URL.Path
+		if strings.HasPrefix(path, "/v1/catalog") || strings.HasPrefix(path, "/v1/health") {
+			b.MaxStaleDuration = s.agent.config.DiscoveryMaxStale


Rewrite this part as follows to make it a bit cleaner

if s.agent.config.DiscoveryMaxState > 0 { b.MaxStaleDuration = s.agent.config.DiscoveryMaxStale b.AllowStale = true }

preetapan · 2018-03-27T16:30:29Z

agent/http.go

+			if b.MaxStaleDuration.Nanoseconds() > 0 {
+				b.AllowStale = true
+			}
+		}


Upon further discussion on this, we thought it would be nice to add a header when the default consistency mode is overridden, so one suggestion we have is to add a new response header. This method could add a header like resp.AddHeader("X-Consul-Effective-Consistency-Level","stale") or resp.AddHeader("X-Consul-Effective-Consistency-Level","leader") etc. That way if a client lib is making a default request to an agent that has DiscoveryMaxStale set and is being overridden by this block, the response should tell you what it used.

Ok, doing it as well

preetapan · 2018-03-27T16:43:02Z

Also please mention the new discovery_max_stale property in the docs here - https://github.com/hashicorp/consul/blob/master/website/source/docs/agent/options.html.md#configuration-key-reference

preetapan · 2018-03-27T16:57:02Z

agent/http.go

+		dur, err := time.ParseDuration(maxStale)
+		if err != nil {
+			resp.WriteHeader(http.StatusBadRequest)
+			fmt.Fprint(resp, "Invalid max_stale time")


Also include the bad value here, so something like fmt.Fprint(resp, "Invalid max_stale value %q", maxStale)

Minor fixes (typos in comments) Removed fmt.Printf debug code Added documentation to new option

pierresouchay · 2018-03-27T21:39:26Z

@preetapan Thank you very much for the review, I made the all the fixes.

The X-Consul-Effective-Consistency-Level is a bit more intrusive than expected since I had to add a setMeta in prepared queries as well (was missing, was maybe a bug)

I also updated the documentation

preetapan · 2018-03-29T19:52:20Z

website/source/docs/agent/options.html.md

@@ -917,6 +917,14 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass
      leader, so this lets Consul continue serving requests in long outage scenarios where no leader can
      be elected.

+    * <a name="discovery_max_stale"></a><a href="discovery_max_stale">`discovery_max_stale`</a> - In a


Rewrite as

`discovery_max_stale` This enables stale requests for all service discovery HTTP endpoints. This is similar to how `max_stale` works for DNS requests. If this value is zero (default) then all service discovery HTTP endpoints are forwarded to the leader. If this value is greater than zero, any Consul server can handle the service discovery request. If a Consul server is behind the leader by more than discovery_max_stale, the query will be re-evaluated on the leader to get more up-to-date results.

preetapan · 2018-03-29T19:55:49Z

agent/structs/structs.go

@@ -157,6 +173,8 @@ type QueryMeta struct {

 	// Used to indicate if there is a known leader node
 	KnownLeader bool
+


Add comment

preetapan · 2018-03-29T22:22:02Z

Did the few last changes in PR #4004 , closing this one out

pierresouchay and others added 21 commits February 26, 2018 17:24

Added a way to override default consistency level per agent

ca9c924

Following conversation hashicorp#3911 This patch allow to change the default behavior of Consul regarding consistency

Allow to have a specific Consistency level for discovery operations

ac6b75f

Added missing fixes in unit tests

523c3ff

Added Unit tests

2fd21a3

Output from test as we run to stop travis killing us and try no paral…

52a7ca8

…lelism to avoid timing issues on their contended CPUs

Only output sparse lines to keep Travis happy while logging verbosely…

dcd3bf5

… to disk

Portability!

726a231

try enabling sudo in Travis to run builds in GCE

aaec358

https://docs.travis-ci.com/user/reference/overview/#Virtualisation-Environment-vs-Operating-System

Try parallel packages but not tests

dc4b0e0

Split the heavy test packages out to their own Jobs.

49102a7

Use relative paths as Travis doesn't setup GOPATH right

1d9e37e

Travis evaluates ENV before cloning git repo and cding so we need to …

b98655e

…delay gathering packages until the makefile

Fix test running in non-bash shells

b666761

remove old pkgs and put deps of missing packages in vendor.json

937d7f4

Update CHANGELOG.md

c1dd2d4

Fixed Unit test TestSanitize

5ba6d78

Added explicit default value in comment

84e4f6a

Added more unit tests

93e01dd

Now use the same mechanism as in DNS with max_stale

0b0e0d4

Merge remote-tracking branch 'origin/master' into default_http_consis…

f3cd6b3

…tency

pierresouchay mentioned this pull request Mar 19, 2018

Watch /catalog/services optimization: X-Consul-Index changes less often #3934

Closed

preetapan suggested changes Mar 27, 2018

View reviewed changes

preetapan reviewed Mar 27, 2018

View reviewed changes

Added X-Consul-Effective-Consistency-Level to discovery requests

6a1cacd

Minor fixes (typos in comments) Removed fmt.Printf debug code Added documentation to new option

Removed unused isAcceptableConsistency func used in previous patch

ab629ab

preetapan reviewed Mar 29, 2018

View reviewed changes

preetapan mentioned this pull request Mar 29, 2018

Adds discovery_max_stale #4004

Merged

preetapan closed this Mar 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a way to override default consistency level per agent #3920

Added a way to override default consistency level per agent #3920

pierresouchay commented Feb 26, 2018

pierresouchay commented Mar 13, 2018

banks commented Mar 14, 2018

preetapan left a comment

preetapan Mar 27, 2018

pierresouchay Mar 27, 2018

preetapan Mar 27, 2018

pierresouchay Mar 27, 2018

preetapan Mar 27, 2018

pierresouchay Mar 27, 2018

preetapan Mar 27, 2018

preetapan Mar 27, 2018

pierresouchay Mar 27, 2018

preetapan commented Mar 27, 2018

preetapan Mar 27, 2018

pierresouchay Mar 27, 2018

pierresouchay commented Mar 27, 2018

preetapan Mar 29, 2018

preetapan Mar 29, 2018

preetapan commented Mar 29, 2018

		@@ -157,6 +173,8 @@ type QueryMeta struct {

		// Used to indicate if there is a known leader node
		KnownLeader bool

Added a way to override default consistency level per agent #3920

Added a way to override default consistency level per agent #3920

Conversation

pierresouchay commented Feb 26, 2018

pierresouchay commented Mar 13, 2018

banks commented Mar 14, 2018

preetapan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

preetapan commented Mar 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierresouchay commented Mar 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

preetapan commented Mar 29, 2018