Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ref(cardinality): Recover buckets on cardinality limiter failure #2852

Merged
merged 2 commits into from
Dec 14, 2023

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Dec 14, 2023

Recover buckets on cardinality limiter error instead of discarding all

#skip-changelog

@Dav1dde Dav1dde requested a review from a team as a code owner December 14, 2023 12:52
@Dav1dde Dav1dde self-assigned this Dec 14, 2023
if !enable_cardinality_limiter {
return Ok(Box::new(buckets.into_iter()));
return Either::Left(buckets.into_iter());
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC Left is for buckets that are accepted and Right for those that are rate-limited. Is this correct? Is it worth documenting it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, but it doesn't have to be like that, Left and Right can contain any iterator, it's just a helper to be able to return 2 distinct iterator types as one concrete type

relay-cardinality/src/limiter.rs Outdated Show resolved Hide resolved
relay-server/src/actors/processor.rs Show resolved Hide resolved
relay-server/src/actors/processor.rs Show resolved Hide resolved
@Dav1dde Dav1dde enabled auto-merge (squash) December 14, 2023 13:28
@Dav1dde Dav1dde merged commit 4fa4081 into master Dec 14, 2023
20 checks passed
@Dav1dde Dav1dde deleted the dav1d/cardinality-error-discards branch December 14, 2023 13:41
jan-auer added a commit that referenced this pull request Dec 19, 2023
* master: (35 commits)
  fix(spans): Parse quotes in MySQL (#2846)
  ref(cardinality): Use a Lua script and in-memory cache for the cardinality limiter (#2849)
  fix(spans): Detect hex with fallback scrubber (#2868)
  release: 23.12.0
  Revert "ci: Update upload-artifact and download-artifact actions" (#2866)
  Revert "build: Update axum and http" (#2863)
  feat(spans): Allow resource.img spans (#2855)
  build: Update axum and http (#2844)
  fix(build): Add additional dependencies to the release build (#2858)
  ci: Update upload-artifact and download-artifact actions (#2861)
  feat(spans): Parse timestamps from strings (#2857)
  fix(spans): Scrub integer file extensions (#2856)
  feat(spans): Remove unused transaction tag from resource metrics (#2853)
  ref(cardinality): Recover buckets on cardinality limiter failure (#2852)
  feat(server): Org rate limit per metric bucket (#2836)
  ref(spans): List metric tags explicitly (#2834)
  feat(spans): Resource response sizes as measurements (#2845)
  feat(crons): Add thresholds to monitor config payload (#2842)
  feat(spans): Allow ingestion of metrics summary on spans (#2823)
  ref(crons): Add documentation to CheckInMessageType (#2840)
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants