Skip to content

fix(taskworker): Clear process-local caches after each task - #127932

Open
marius-mbravio wants to merge 1 commit into
getsentry:masterfrom
marius-mbravio:fix/taskworker-clear-local-caches
Open

marius-mbravio wants to merge 1 commit into
getsentry:masterfrom
marius-mbravio:fix/taskworker-clear-local-caches

Conversation

@marius-mbravio

Copy link
Copy Markdown

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 shell built configs with the new setting, while configs built by the taskworker kept the old one until each worker process was replaced. Tracing it back:

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.

  • LocalCacheClearHook in sentry/taskworker/adapters.py, registered in runtime.py next to ViewerContextHook. taskbroker-client enters every hook's on_execute context around each task (_execute_activation, ExitStack), so the hook clears on exit, whether the task succeeded or failed.
  • It clears what task_postrun used to clear: the OrganizationOption, ProjectOption, UserOption and GroupMeta local caches, and calls options.default_store.maybe_clean_local_cache(). A failure to clear is logged and never fails the task.
  • No Celery dependency; it uses only the taskbroker-client hook interface.

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):

  • A task reads organization and project options; another process then saves new values (database and shared cache); the next task in the same worker reads the new values.
  • The option caches are empty after a task finishes, and after a task fails.
  • GroupMeta and the options store are cleared too.
  • A failure to clear does not fail the task.
  • The Sentry task app registers the hook.

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.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant