Repository navigation
fix(taskworker): Clear process-local caches after each task - #127932
Open
marius-mbravio wants to merge 1 commit into
Open
marius-mbravio wants to merge 1 commit into
marius-mbravio wants to merge 1 commit into
Conversation
Web requests clear the option managers' process-local caches on Django's `request_finished`. Tasks used to clear them on Celery's `task_postrun`, which stopped firing when tasks moved to taskworker; the last connections were removed as dead Celery code in getsentry#100326. Since then a long-running taskworker keeps the organization, project and user options it read in an earlier task and uses them for every later one, until the process is replaced. Project configs built for Relay were one visible effect (getsentry#127554). LocalCacheClearHook restores the per-task clear as a taskworker ContextHook, next to ViewerContextHook. When a task finishes, successfully or not, it clears what `task_postrun` used to clear: the OrganizationOption, ProjectOption, UserOption and GroupMeta local caches, and the options store's periodic local cleanup. A failure to clear is logged and never fails the task. Refs getsentry#127554
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A long-running taskworker never clears the option managers' process-local caches, so every task after the first uses the organization, project and user options an earlier task read. This restores the per-task clear Celery used to provide, as a taskworker
ContextHook.Refs #127554. Related: #127930, #127929.
How we got here. We filed #127554 after data scrubbing changes on our self-hosted Sentry (26.7.2) never reached Relay: a fresh
sentry shellbuilt configs with the new setting, while configs built by the taskworker kept the old one until each worker process was replaced. Tracing it back:OptionManager._option_cache, athreading.local). Web requests clear it on Django'srequest_finished.task_postrun, connected since 2012 (e8fc407, "Bind to task_postrun for clearing option caches").task_postrunstopped firing for them. chore(tasks) Remove more celery related code pt.3 #100326 (2025-09-26, removing Celery code) then removed the connections as dead code: inOptionManager,GroupMeta, and the options store. Nothing replaced them.OrganizationOption.objects.reload_task_local_cacheincompute_projectkey_configfor one visible effect, stale organization options in project configs. That reload only drops the local copy when the shared cache is empty, which is Data scrubbing changes never reach Relay: taskworker rebuilds project configs from stale organization options #127554's immediate bug, fixed for organization options by fix(relay): Refresh task-local organization options from the shared c… #127929 and for organization and project options by fix(relay): Refresh project and organization options before computing a config #127930.Those PRs fix the Relay config path. The cause is wider: any task can read options cached by an earlier task in the same worker, for organization, project and user options alike, until the worker is replaced (
--max-child-task-count).Change.
LocalCacheClearHookinsentry/taskworker/adapters.py, registered inruntime.pynext toViewerContextHook. taskbroker-client enters every hook'son_executecontext around each task (_execute_activation,ExitStack), so the hook clears on exit, whether the task succeeded or failed.task_postrunused to clear: theOrganizationOption,ProjectOption,UserOptionandGroupMetalocal caches, and callsoptions.default_store.maybe_clean_local_cache(). A failure to clear is logged and never fails the task.Cost. Each task's first read of an instance's options goes to the shared cache again instead of process memory, which is how tasks behaved under Celery.
Tests (
tests/sentry/taskworker/test_adapters.py):GroupMetaand the options store are cleared too.Removing the clear fails four of these; dropping the registration fails the registration test.
tests/sentry/taskworker/passes (103), and prek and mypy are clean on the changed files.Relation to #127930 and #127929. With this hook, configs built in a new task read fresh options, so the targeted reloads become redundant for that path but stay harmless; they still help within one long task that builds many configs. Whether to take this, the targeted fix, or both is the team's call.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.