Skip to content

[handler] Fix rate limiting behavior#40

Merged
harveyxia merged 1 commit intomainfrom
fix-observed-event-handler
May 1, 2025
Merged

[handler] Fix rate limiting behavior#40
harveyxia merged 1 commit intomainfrom
fix-observed-event-handler

Conversation

@harveyxia
Copy link
Copy Markdown
Collaborator

Overview

This bug was introduced in this PR—we accidentally updated the observed event handler to rate limit all events, whereas controller-runtime's behavior is to only rate limit error and requeue (without specified delays) results.

Implications

Controllers subject to this bug, if rate limits are actively delaying new work from being enqueued, will see unexpected delays from when a trigger event fires in the cluster to when it is processed by the controller. Normally, triggers other than errors and requeues (without specified delays) should be enqueued immediately.

Worst case this behavior results in downstream user/system impact where they experience undue latency from a spec change to when it's instantiated.

This is the exact behavior I saw described here, which I never found the root cause of. With this one line change, the test succeeds, giving me confidence that this is the root cause of unexpected delays in processing.

@harveyxia harveyxia requested a review from a team as a code owner May 1, 2025 14:09
Copy link
Copy Markdown
Contributor

@shadialtarsha shadialtarsha left a comment

Choose a reason for hiding this comment

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

Oops 😬

@harveyxia harveyxia merged commit 7060e46 into main May 1, 2025
1 check passed
@harveyxia harveyxia deleted the fix-observed-event-handler branch May 1, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants