[Streaming] Results from nodes with streaming set header X-Consul-Knownleader: false

[pierresouchay]

When Streaming is running, the header X-Consul-Knownleader: false is set to false.

Some SDKs such as Orbitz, use this field to throw an error, thus, it fails to retrieve data.

This header should be set to true with streaming activated

[pierresouchay]

Cc @rickfast, @heruv1m, @yfouquet any opinion on this (and the linked PR and discussion)?

[chuckyz]

@pierresouchay I encountered a situation in a very small production DC where X-Consul-Knownleader: false was appended only on a specific X-Consul-Index which ended up breaking all communication from a client perspective in one case (e.g.: curl http://127.0.0.1/v1/health/service/foo?index=12345 returned a 500 but curl http://127.0.0.1/v1/health/service/foo?index=12346 did not). I've only ever seen this happen with grpc streaming. Could this possibly be related?

I saw your issue and was curious if anyone else has seen the above behavior.

[pierresouchay]

@chuckyz Unlikely, in such case, the Consul agent does not return HTTP 500, it does return HTTP 200, but with such header wrongly set (which is interpreted by Orbitz as no-result). So, if you are sure it was an HTTP 500, it was not the same thing.

You could have however hit another but fixed (in our case) by #9555 when a service is not changing too much (I mean less than every 10min), which is likely the case if you are using a vanilla Consul without any patch. See https://medium.com/criteo-engineering/consul-streaming-whats-behind-it-6f44f77a5175

[dnephin]

Cross posting from #9778,

The Consistency Modes docs for this header say:

can be used by clients to gauge the staleness of a result and take appropriate action

A false value for this header does not mean "there is no leader", it simply means that the leader status was not verified. There are plenty of places where we return KnownLeader: false simply because we didn't set the field, and when the response is returned from the cache we may set it to true, even though the leader status was not verified as part of that request.

It does not seem appropriate to error when this header is false.

[pierresouchay]

Hello @dnephin ,

I highly disagree with your analysis of the doc and about who is wrong/right. From an implementor POV, this field means : the data you see is not correct, hence the choice of Orbitz to say: this data cannot be trusted. So it seems reasonable to make the choice of failing.

There are plenty of places where we return KnownLeader: false simply because we didn't set the field

That's not my experience : it happens only IIRC where there are some calls to Consul Servers, even so, having this field set, from an implementor POV means don't trust this data. I think implementors of SDKs cannot be blamed for that, because it sounds reasonable. This field not being set in Consul internals is not an valid excuse IMHO. Either set it to true, either discard it. But having it false in API responses is a breaking change to me, blowing away existing semantics.

It simply means that streaming breaks the API (and it did for us at large scale when I was working at Criteo) in some kind of subtle ways, and this is really dangerous.

Cc @yfouquet

[dnephin]

I am not sure how it implies the data is incorrect. At most it seems like it could imply the data is maybe stale. It can't even say definitely if it is stale or not, because if the data hasn't changed on the leader it is not stale.

I can definitely say that it is misleading in many cases. If any response is returned from the cache this value could be true, but the value in the cache could be up to 3 days old, so a leader could have been lost any time during those 3 days and it will still say KnownLeader: true anyway.

I agree with you that subtle changes like this to the API are a problem. A safer approach would be to completely remove the header from the API entirely, since it can't be used safely. That would require a change to any SDK anyway, so probably ignoring the header for now is a good approach until it can be removed.