Skip to content

fix(dynamic-sampling): transaction management error#108117

Merged
shellmayr merged 3 commits intomasterfrom
shellmayr/fix/transaction-management-error-dynamic-sampling
Feb 16, 2026
Merged

fix(dynamic-sampling): transaction management error#108117
shellmayr merged 3 commits intomasterfrom
shellmayr/fix/transaction-management-error-dynamic-sampling

Conversation

@shellmayr
Copy link
Member

@shellmayr shellmayr commented Feb 12, 2026

Bug explanation:

  • When in a transaction, in_atomic_block is true for the connection object
  • When in_atomic_block is true, but autocommit is off, Django does not know when changes will be committed, so we cannot register the callback

Fix:

  • If not in a transaction, call the scheduling function directly
  • If in a transaction, call it as a callback via on_commit

Fixes SENTRY-5GVE
Fixes SENTRY-5HMP
Fixes SENTRY-5HMN
Closes TET-1724

@shellmayr shellmayr requested a review from a team as a code owner February 12, 2026 13:46
@linear
Copy link

linear bot commented Feb 12, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 12, 2026
@shellmayr shellmayr requested a review from a team February 12, 2026 13:46
@constantinius constantinius changed the title Shellmayr/fix/transaction management error dynamic sampling fix: transaction management error dynamic sampling Feb 12, 2026
@shellmayr shellmayr changed the title fix: transaction management error dynamic sampling fix(dynamic-sampling): transaction management error Feb 12, 2026
Copy link
Contributor

@constantinius constantinius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! It could still be possible that the issue is not fixed with that when the scheduling is done in an atomic block.

Comment on lines +322 to +328
if (
options.get("relay.invalidation-direct-outside-atomic")
and not connections[transaction_db].in_atomic_block
):
_do_schedule()
else:
transaction.on_commit(_do_schedule, using=transaction_db)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth putting this in a custom on_commit function, basically replicate Django's on_commit without the self.get_autocommit() branch:

https://github.com/django/django/blob/fee2cb2d6dfefad614b27e659f5648c793006ef8/django/db/backends/base/base.py#L730-L736

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer doing it like this for now - it's clearer what happens instead of "hiding" this switching functionality in a custom on_commit

@shellmayr shellmayr merged commit 955a559 into master Feb 16, 2026
100 checks passed
@shellmayr shellmayr deleted the shellmayr/fix/transaction-management-error-dynamic-sampling branch February 16, 2026 09:19
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.

3 participants

Comments