fix(group_update): Handle last_seen integer overflow on groups by yuvmen · Pull Request #104297 · getsentry/sentry

yuvmen · 2025-12-02T23:45:41Z

Using error upsampling groups can cross the MAX_INT value of last_seen, handle these errors and cap the value to deal with these errors for now.

codecov · 2025-12-02T23:46:51Z

Codecov Report

❌ Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/sentry/buffer/base.py	86.95%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #104297    +/-   ##
=========================================
  Coverage   80.57%    80.58%            
=========================================
  Files        9340      9337     -3     
  Lines      399123    399487   +364     
  Branches    25553     25553            
=========================================
+ Hits       321581    321912   +331     
- Misses      77093     77126    +33     
  Partials      449       449

Using error upsampling groups can cross the MAX_INT value of last_seen, handle these errors and cap the value to deal with these errors for now.

the update dict actually already holds other values than experssion, but the direct assignment we added causes it to error. Instead of creating a complex type for the update dict just set it to Any

wedamija · 2025-12-03T23:49:08Z

src/sentry/buffer/base.py

+                            update_kwargs["times_seen"] = MAX_INT32
+                            try:
+                                group.update(using=None, **update_kwargs)
+                                logger.warning(


Could be worth logging this to datadog instead. I think that warnings will end up as sentry errors too, so you might want to drop these to info.

ohh right actually these would just spam, I guess a datadog metric is the cheapest way to keep track of it?

went ahead and replaced with metric, and added a metric for the retry failure as well

wedamija · 2025-12-03T23:49:40Z

src/sentry/buffer/base.py

+                                )
+                            except Exception as retry_error:
+                                # If the capped update also fails, log and skip
+                                logger.warning(


Probably worth logging as an error if this fails

mrduncan · 2025-12-04T00:14:56Z

src/sentry/buffer/base.py

                else:
-                    group.update(using=None, **update_kwargs)
+                    try:
+                        group.update(using=None, **update_kwargs)


Curious whether we could just do something like this to save a db roundtrip?

if "times_seen" in update_kwargs: update_kwargs["times_seen"] = min(update_kwargs["times_seen"], MAX_INT32)

yea I thought about it as well, and I did decide to go this route to maintain visibility and keep track of when this happens... but thinking about it some more I dont know if its worth the round trip and the ever increasing metric just to be able to tell this is going on. We will always be able to check for groups with MAX_INT last_seen to know which are affected. It felt like bad practice but now I am actually thinking this is the cleaner option, I think ill change to do this.

ah ok its not that simple, we dont use the times_seen value, we just tell it to basically inc the existing value - giving it an expression of like V + 1 to handle race conditions and such. Looks like there is no easy way to do it at the DB level without overflowing, the only way is using Case which would really slow down updates for all groups and I am reluctant to do that. I think keeping updates fast for the 99.9% case and slow for these groups might be better.
However we can actually just add a check for the current last_seen value and if it is MAX_INT, not update times_seen, which would prevent the overflow

Is Least helpful here?

I think something like

for c, v in columns.items(): if c == "times_seen": update_kwargs[c] = Least(F(c) + v, MAX_INT32)

runs atomically to set times_seen as the smallest of the values provided

yea I tried, but it turns out Least() has to actually calculate that value, and it overflows and errors in the DB when it does that, so no dice

the only thing that can be done in DB level that I found is adding a CASE statement, but that would really slow down updates, even Least would but I would have accepted that I think

…ady maxed

thetruecpaul

Approving with nits.

This is better than current state, but I think (either before landing or as a followup) I'd suggest you look into @shashjar's comment about Least. Obviously we can't do the impossible, but if there is a way to do this in fewer DB hits then that'd be awesome

thetruecpaul · 2025-12-05T17:35:44Z

src/sentry/buffer/base.py

+logger = logging.getLogger(__name__)
+
+# Maximum value for a 32-bit signed integer
+MAX_INT32 = 2_147_483_647


Nit: could you use BoundedPositiveIntegerField.MAX_VALUE instead of a new constant to avoid duplication & match the Group field definition?

thetruecpaul · 2025-12-05T17:38:23Z

src/sentry/buffer/base.py


        if not signal_only:
-            update_kwargs: dict[str, Expression] = {c: F(c) + v for c, v in columns.items()}
+            update_kwargs: dict[str, Any] = {c: F(c) + v for c, v in columns.items()}


Nit: Can we keep the type as constrained as possible? Maybe dict[str, Expression | int] instead of opening the door to Any?

the original Expression is actually not accurate and that dict does not contain only Expression, mypy just doesnt detect it but the update_kwargs contains other things than Expression, the real type is more convoluted and it actually not enforced so I wanted it to be clear here. The extra dict can contain datetime or any other arbitrary type, so Any is probably warranted here.

src/sentry/buffer/base.py

…tead of columns

Using error upsampling groups can cross the MAX_INT value of last_seen, handle these errors and cap the value to deal with these errors for now.

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 2, 2025

vercel bot deployed to Preview December 2, 2025 23:48 View deployment

fix(group_update): Handle last_seen integer overflow on groups

bbdaa31

Using error upsampling groups can cross the MAX_INT value of last_seen, handle these errors and cap the value to deal with these errors for now.

yuvmen force-pushed the yuvmen/handle_lastseen_integer_overflow branch from dc9280d to bbdaa31 Compare December 3, 2025 20:04

vercel bot deployed to Preview December 3, 2025 20:07 View deployment

fix typing issue

b589963

the update dict actually already holds other values than experssion, but the direct assignment we added causes it to error. Instead of creating a complex type for the update dict just set it to Any

vercel bot deployed to Preview December 3, 2025 21:14 View deployment

yuvmen marked this pull request as ready for review December 3, 2025 22:54

improved error condtion to use error code + added test

bbaa7e5

yuvmen requested a review from a team December 3, 2025 23:16

vercel bot deployed to Preview December 3, 2025 23:17 View deployment

wedamija reviewed Dec 3, 2025

View reviewed changes

replaced warning log with metric on buffer overflow

8ea2691

mrduncan reviewed Dec 4, 2025

View reviewed changes

vercel bot deployed to Preview December 4, 2025 00:17 View deployment

added a check to not attempt an increment to times_seen if it is alre…

3f3717c

…ady maxed

vercel bot deployed to Preview December 4, 2025 18:50 View deployment

yuvmen requested review from mrduncan and wedamija December 4, 2025 18:54

thetruecpaul approved these changes Dec 5, 2025

View reviewed changes

replace MAX_INT32 with BoundedPositiveIntegerField.MAX_VALUE

09cfbc2

vercel bot deployed to Preview December 5, 2025 18:03 View deployment

shashjar approved these changes Dec 5, 2025

View reviewed changes

cursor bot reviewed Dec 5, 2025

View reviewed changes

src/sentry/buffer/base.py Outdated Show resolved Hide resolved

changed exception condition to find times_seen in update_kwargs ins…

4554cc3

…tead of columns

vercel bot deployed to Preview December 5, 2025 18:15 View deployment

yuvmen enabled auto-merge (squash) December 5, 2025 18:23

yuvmen merged commit 55abbe6 into master Dec 5, 2025
67 checks passed

yuvmen deleted the yuvmen/handle_lastseen_integer_overflow branch December 5, 2025 18:35

github-actions bot locked and limited conversation to collaborators Dec 21, 2025

Uh oh!

Conversation

yuvmen commented Dec 2, 2025

Uh oh!

codecov bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuvmen Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thetruecpaul left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Dec 2, 2025 •

edited

Loading

yuvmen Dec 4, 2025 •

edited

Loading