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

various date/time-related fixes #75625

Closed
wants to merge 5 commits into from

Conversation

rockdrilla
Copy link

No description provided.

@rockdrilla rockdrilla requested review from a team as code owners August 6, 2024 11:26
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 6, 2024
Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

Sorry, these migrations can't be run on our production systems because they'll require a full table rewrite. Is there a reason you'd like to increase the size of these columns?

@rockdrilla
Copy link
Author

Unfortunately, we're hitting integer overflows on these columns in our self-hosted setup which leads to sentry workers getting stuck and [other] queues growing with blazing speed.

"abstime" type was removed in that release, see notes:
https://www.postgresql.org/docs/12/release-12.html

Signed-off-by: Konstantin Demin <rockdrilla@gmail.com>
Signed-off-by: Konstantin Demin <rockdrilla@gmail.com>
this is merely a mixin of BoundedPositiveIntegerField and
BoundedBigIntegerField.

Signed-off-by: Konstantin Demin <rockdrilla@gmail.com>
this field/column easily triggers integer overflow.

Signed-off-by: Konstantin Demin <rockdrilla@gmail.com>
this field/column may trigger integer overflow.
NB: "times_seen" is indexed field, so issue REINDEX TABLE after changing column type.

Signed-off-by: Konstantin Demin <rockdrilla@gmail.com>
@rockdrilla
Copy link
Author

@wedamija can't access anymore your reviewed changes after force-pushing branch. May I ask you to review once more?

@wedamija
Copy link
Member

Unfortunately, we're hitting integer overflows on these columns in our self-hosted setup which leads to sentry workers getting stuck and [other] queues growing with blazing speed.

Does this mean you have group(s) in your table that have seen over 2 biliion events? In our own production setup, there are none so far with over 1 billion.

Are you able to tell me the maximum value of both the times_seen and score column in your sentry_groupedmessages table?

@wedamija
Copy link
Member

wedamija commented Aug 12, 2024

@wedamija can't access anymore your reviewed changes after force-pushing branch. May I ask you to review once more?

My main comment is that we can't merge this as is without a lot of work on our end - upgrading the size of a column from an int to a bigint requires a full table lock and will take down our production systems until the rewrite is completed. For us to manage this safely we'd need to figure out a way to manage this change without locking the table.

I'm happy to work with you to try and fix the problems in your own system though.

@getsantry
Copy link
Contributor

getsantry bot commented Sep 3, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Sep 3, 2024
@getsantry getsantry bot closed this Sep 11, 2024
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 Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants