Skip to content
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

perf(nodestore): disable setting cache on write #68439

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Apr 8, 2024

We are setting cache on every transaction, but not always reading it. This optimization would move the setting cache operation to read.

I've moved the set_cache_item call inside an option nodestore.set_subkeys.enable_set_cache_item in case we need to revert this.

Ref: https://github.com/getsentry/team-processing/issues/135

cc @getsentry/processing

@anonrig anonrig requested review from mitsuhiko, markstory and a team April 8, 2024 15:07
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 8, 2024
@anonrig anonrig requested a review from fpacifici April 8, 2024 15:08
@anonrig anonrig force-pushed the anonrig/disable-set-cache-nodestore branch from 29c7824 to e213403 Compare April 8, 2024 15:09
src/sentry/options/defaults.py Outdated Show resolved Hide resolved

This comment was marked as off-topic.

@anonrig anonrig force-pushed the anonrig/disable-set-cache-nodestore branch from ab70a89 to 33a6e81 Compare April 8, 2024 18:50
@anonrig anonrig merged commit 549848b into master Apr 8, 2024
48 checks passed
@anonrig anonrig deleted the anonrig/disable-set-cache-nodestore branch April 8, 2024 20:05
shellmayr pushed a commit that referenced this pull request Apr 10, 2024
We are setting cache on every transaction, but not always reading it.
This optimization would move the setting cache operation to `read`.

I've moved the `set_cache_item` call inside an option
`nodestore.set_subkeys.enable_set_cache_item` in case we need to revert
this.

Ref: getsentry/team-processing#135

cc @getsentry/processing
Copy link

sentry-io bot commented Apr 14, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures... pytest.runtest.protocol tests/sentry/nodestore/... View Issue

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants