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

Manual backport of #17775 #17781

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Conversation

hashi-derek
Copy link
Member

Fix issue with streaming service health watches.

This commit fixes an issue where the health streams were unaware of service
export changes. Whenever an exported-services config entry is modified, it is
effectively an ACL change.

The bug would be triggered by the following situation:

- no services are exported
- an upstream watch to service X is spawned
- the streaming backend filters out data for service X (due to lack of exports)
- service X is finally exported

In the situation above, the streaming backend does not trigger a refresh of its
data.  This means that any events that were supposed to have been received prior
to the export are NOT backfilled, and the watches never see service X spawning.

We currently have decided to not trigger a stream refresh in this situation due
to the potential for a thundering herd effect (touching exports would cause a
re-fetch of all watches for that partition, potentially).  Therefore, a local
blocking-query approach was added by this commit for agentless.

It's also worth noting that the streaming subscription is currently bypassed
most of the time with agentful, because proxycfg has a `req.Source.Node != ""`
which prevents the `streamingEnabled` check from passing.  This means that while
agents should technically have this same issue, they don't experience it with
mesh health watches.

Note that this is a temporary fix that solves the issue for proxycfg, but not
service-discovery use cases.
@hashi-derek hashi-derek marked this pull request as ready for review June 15, 2023 19:40
@hashi-derek hashi-derek requested a review from mkeeler June 15, 2023 19:42
@hashi-derek hashi-derek merged commit 5bf4381 into release/1.16.0 Jun 15, 2023
@hashi-derek hashi-derek deleted the derekm/NET-4506/release-1.16.0 branch June 15, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants