[FIX] Increase backoff on Quarterdeck JWKS sync #24
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds explicit exponential backoff configuration to the Quarterdeck JWKS synchronization to address rate limiting errors (SC-35715). Previously, the backoff used library defaults; now specific parameters are configured including initial interval, multiplier, randomization factor, and maximum interval.
Key Changes:
- Added four new backoff configuration constants (InitialInterval, RandomizationFactor, Multiplier, MaxInterval)
- Modified the
Sync()method to explicitly configure exponential backoff with these constants - Added a clarifying comment about the critical section in
GetKey()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| delay := backoff.NewExponentialBackOff() | ||
| delay.InitialInterval = BackoffInitialInterval | ||
| delay.RandomizationFactor = BackoffRandomizationFactor | ||
| delay.Multiplier = BackoffMultiplier | ||
| delay.MaxInterval = BackoffMaxInterval |
There was a problem hiding this comment.
The PR description states this change "increases the backoff retry delay" to address rate limiting, but the MaxInterval of 30 seconds may actually be shorter than typical exponential backoff defaults (which are often 60+ seconds or unlimited). With the current settings, retries will occur at: 1s, 2s, 4s, 8s, 16s, 30s, 30s, etc., reaching the maximum delay after only 5 retries. For rate limiting issues, consider either increasing MaxInterval (e.g., to 60 seconds or more) or increasing InitialInterval to provide more breathing room between retries, ensuring the backoff truly increases as intended.
| return nil, auth.ErrInvalidKeyID | ||
| } | ||
|
|
||
| // Critical Section: only called from Verify which has a read lock |
There was a problem hiding this comment.
The comment states "only called from Verify" but GetKey is also called from the Refresh method (line 162), which also holds a read lock. Consider updating the comment to be more accurate, for example: "Critical Section: called from Verify and Refresh, both holding a read lock".
| // Critical Section: only called from Verify which has a read lock | |
| // Critical Section: called from Verify and Refresh, both holding a read lock |
Scope of changes
Increases the backoff retry delay on Quarterdeck JWKS syncs from Endeavor in an effort to repair rate limiting errors.
Fixes SC-35715
Type of change
Acceptance criteria
This PR will be merged without review.
Definition of Done