-
Notifications
You must be signed in to change notification settings - Fork 93
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
fix(projects): Treat fetch failures as pending #4140
Conversation
time.sleep(1) # Wait for project to be cached | ||
|
||
# Relay still accepts events for this project | ||
next_response = relay.send_event(42) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the old version, this call raises a 403 error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does have the downside of potentially exhausting buffers if a project consistently cannot be computed. Do we need more alerts for this case?
Good point, we should alert when the buffer gets close to its total capacity, or implement @iambriccardo 's automatic eviction policy. I added a task to the epic. |
Note: not merging this until https://github.com/getsentry/team-ingest/issues/345 is mature. |
Measure impact before we merge #4140.
688d23e
to
2970ed0
Compare
When we fail to fetch a project state for any reason (max retries exceeded, redis error), we should not mark the project as invalid.
Currently we treat the project as if the DSN was invalid, and respond to clients with a 403 response. This might have made sense as a defensive measure in a pre-spooling world where envelopes could not be buffered for a longer period of time, but with spooling and time-based envelope eviction, we can now safely buffer them.
ref: INC-888
blocked by: https://github.com/getsentry/team-ingest/issues/345