Repository navigation
fix(relay): Flush the organization options local cache for project recomputation - #117602
Conversation
github-actions
Bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7527fb2. Configure here.
| # value with an actual query. | ||
| result = cache.get(cache_key) | ||
| if result is None and cache_key in self._option_cache: | ||
| del self._option_cache[cache_key] |
There was a problem hiding this comment.
Local cache not flushed
High Severity
reload_task_local_cache only removes a task-local entry when the shared cache.get returns None. After the monolith refreshes organization options in the shared cache, a stale _option_cache entry can remain, and get_all_values returns it without re-reading shared storage—so relay invalidate/build tasks may compute project configs from outdated organization options.
Reviewed by Cursor Bugbot for commit 7527fb2. Configure here.
| result = cache.get(cache_key) | ||
| if result is None and cache_key in self._option_cache: | ||
| del self._option_cache[cache_key] |
There was a problem hiding this comment.
Bug: The reload_task_local_cache method fails to clear the local cache (_option_cache) when the shared cache has been updated, causing subsequent operations to use stale data.
Severity: MEDIUM
Suggested Fix
Modify reload_task_local_cache to ensure the local cache entry is always invalidated, regardless of the state of the shared cache. The simplest fix is to remove the result is None condition and unconditionally delete the key from self._option_cache if it exists. This will force get_all_values to reload the data from the shared cache on its next call.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/models/options/organization_option.py#L82-L84
Potential issue: The `reload_task_local_cache` method fails to invalidate a stale,
thread-local cache (`_option_cache`) when the shared cache has been updated with fresh
data. The method only clears the local cache entry if the shared cache returns `None`.
In the intended scenario where an organization's options are updated, the shared cache
contains the new value (is not `None`), causing the invalidation logic to be skipped.
Consequently, long-running celery workers that have previously cached an organization's
options will continue to use the stale local data for subsequent tasks, leading to
incorrect project configurations.
Dav1dde
left a comment
Dav1dde
left a comment
There was a problem hiding this comment.
As discussed there is a potential invocation which may be missed, but that may as well be an improvement for the future.
| result = cache.get(cache_key) | ||
| if result is None and cache_key in self._option_cache: | ||
| del self._option_cache[cache_key] |
There was a problem hiding this comment.
Bug: The reload_task_local_cache method fails to update the local cache when the shared cache has a newer value, causing stale data to be used.
Severity: HIGH
Suggested Fix
Modify reload_task_local_cache to ensure the local cache is always invalidated or updated, regardless of the state of the shared cache. One approach is to always delete the entry from self._option_cache if it exists. Another is to update self._option_cache[cache_key] = result when result is not None.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/models/options/organization_option.py#L82-L84
Potential issue: The `reload_task_local_cache` method is intended to refresh a
thread-local cache (`_option_cache`) from a shared cache. However, it only clears the
local cache entry if the shared cache is empty (`result is None`). If the shared cache
has a newer value than the local cache, the method does nothing. Consequently,
subsequent calls to `get_all_values` will read the stale data from the local cache
without ever checking the updated shared cache. This can happen when an organization's
options are updated in a separate process while a background task is running, leading
the task to operate on outdated configuration.
…computation (#117602) The local OrganizationOption cache (_options_cache) is never flushed during project config updates/builds (within a task--it is correctly flushed in the monolith.) Add a blanket reload within the invalidate and build tasks.
3 participants
The local OrganizationOption cache (_options_cache) is never flushed during project config updates/builds (within a task--it is correctly flushed in the monolith.) Add a blanket reload within the invalidate and build tasks.