feat: limits-frontend failover to other zones #17408

grobinson-grafana · 2025-04-23T11:24:47Z

What this PR does / why we need it:

This pull request updates the limits-frontend to failover to other zones for stream hashes that cannot be queried in the current zone.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
Title matches the required conventional commits format, see here
- Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

grobinson-grafana · 2025-04-23T12:47:29Z

pkg/limits/frontend/ring.go

+		// can use sort.Search to subtract the two slices.
+		slices.Sort(streamHashesToDelete)
+		streamHashesToQuery = slices.DeleteFunc(streamHashesToQuery, func(streamHashToQuery uint64) bool {
+			// see https://pkg.go.dev/sort#Search


This should be O(N + logN)

grobinson-grafana · 2025-04-23T16:26:06Z

pkg/limits/frontend/ring.go

+			return responses, nil
+		}
+	}
+	// Treat remaining stream hashes as unknown streams.


I think there is an interesting question here of is this the correct behavior, or should this be an error?

In the case where some subset of stream hashes cannot be queried because either:

There are no pods assigned as consumers for the partition that owns the stream hashes

All pods across all zones that consume this partition were unavailable

This subset of stream hashes will be treated as unknown streams. Is this the correct thing to do?

I believe the correct behavior here is to return an error because:

We classify only two non ambiguous types or unknown streams: exceeds_max_streams and exceeds_rate_limit. Adding a third one for "could-not-query` is too ambiguous.

Even if we return the streams as unknown back it is not easy to tell the distributor clients what to do. Should they just retry? Or bubble up an error to the agents?

grobinson-grafana · 2025-05-27T14:34:30Z

pkg/limits/frontend/ring.go

+			break
+		}
+	}
+	// TODO(grobinson): In a subsequent change, I will figure out what to do


At present, unanswered streams are permitted. First I want to emit a metric to track this, and I will do this in a follow up PR to make the change easier to see.

This commit updates the limits-frontend to failover to other zones for stream hashes that cannot be queried in the current zone.

periklis · 2025-05-28T08:59:16Z

pkg/limits/frontend/mock_test.go

 	numAssignedPartitionsRequests int
 	numExceedsLimitsRequests      int


Double checking, no need for a mutex for these? no concurrent test cases?

No mutex required right now. Each test case uses a separate mock, and requests are executed in sequence.

periklis · 2025-05-28T09:06:10Z

pkg/limits/frontend/ring.go

+		if err != nil {
+			continue
+		}
+		responses = append(responses, resps...)


nit.

Suggested change

responses = append(responses, resps...)

responses = append(responses, resps...)

if len(streams) == len(answered) {

break

}

We also need to delete the streams, can't just break, as we will check len(streams) later to see if some streams went unanswered.

Going to look at this in the next PR 👍

grobinson-grafana requested a review from a team as a code owner April 23, 2025 11:24

pull-request-size bot added the size/L label Apr 23, 2025

grobinson-grafana commented Apr 23, 2025

View reviewed changes

grobinson-grafana force-pushed the grobinson/failover-to-other-zones branch 3 times, most recently from 66c02a0 to 57ccf97 Compare April 23, 2025 16:25

grobinson-grafana commented Apr 23, 2025

View reviewed changes

grobinson-grafana force-pushed the grobinson/failover-to-other-zones branch 3 times, most recently from e2f6226 to 37b2513 Compare April 23, 2025 20:22

grobinson-grafana force-pushed the grobinson/failover-to-other-zones branch 7 times, most recently from 4072cd3 to 71eff46 Compare May 27, 2025 14:28

grobinson-grafana commented May 27, 2025

View reviewed changes

grobinson-grafana force-pushed the grobinson/failover-to-other-zones branch from 71eff46 to b5553fb Compare May 27, 2025 18:26

pull-request-size bot added size/XL and removed size/L labels May 27, 2025

grobinson-grafana force-pushed the grobinson/failover-to-other-zones branch 2 times, most recently from 620e79a to b9480bb Compare May 27, 2025 18:36

feat: limits-frontend failover to other zones

c0ab884

This commit updates the limits-frontend to failover to other zones for stream hashes that cannot be queried in the current zone.

grobinson-grafana force-pushed the grobinson/failover-to-other-zones branch from b9480bb to c0ab884 Compare May 28, 2025 08:27

Fix incorrect stream hash

6d38041

periklis approved these changes May 28, 2025

View reviewed changes

grobinson-grafana merged commit 93e829a into main May 28, 2025
65 checks passed

grobinson-grafana deleted the grobinson/failover-to-other-zones branch May 28, 2025 09:58

loki-gh-app bot mentioned this pull request Jul 28, 2025

chore(k265): release 3.5.0 #18605

Open

This was referenced Aug 5, 2025

chore(k266): release 3.5.0 #18722

Open

chore(k267): release 3.5.0 #18796

Open

loki-gh-app bot mentioned this pull request Aug 18, 2025

chore(k268): release 3.5.0 #18891

Open

loki-gh-app bot mentioned this pull request Sep 1, 2025

chore(k270): release 3.5.0 #19075

Open

loki-gh-app bot mentioned this pull request Sep 8, 2025

chore(k271): release 3.5.0 #19126

Open

loki-gh-app bot mentioned this pull request Sep 15, 2025

chore(k272): release 3.5.0 #19194

Open

loki-gh-app bot mentioned this pull request Sep 22, 2025

chore(k273): release 3.5.0 #19248

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: limits-frontend failover to other zones #17408

feat: limits-frontend failover to other zones #17408

Uh oh!

grobinson-grafana commented Apr 23, 2025 •

edited

Loading

Uh oh!

grobinson-grafana Apr 23, 2025

Uh oh!

grobinson-grafana Apr 23, 2025

Uh oh!

grobinson-grafana Apr 23, 2025 •

edited

Loading

Uh oh!

periklis Apr 25, 2025

Uh oh!

grobinson-grafana May 27, 2025

Uh oh!

periklis May 28, 2025

Uh oh!

grobinson-grafana May 28, 2025

Uh oh!

periklis May 28, 2025

Uh oh!

grobinson-grafana May 28, 2025

Uh oh!

grobinson-grafana May 28, 2025

Uh oh!

Uh oh!

Uh oh!

		numAssignedPartitionsRequests int
		numExceedsLimitsRequests int

feat: limits-frontend failover to other zones #17408

feat: limits-frontend failover to other zones #17408

Uh oh!

Conversation

grobinson-grafana commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grobinson-grafana Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grobinson-grafana commented Apr 23, 2025 •

edited

Loading

grobinson-grafana Apr 23, 2025 •

edited

Loading