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

Add support for files up to 4G #88882

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for files up to 4G #88882

wants to merge 1 commit into from

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Apr 7, 2025

This is achieved by introducing a new WrappingU32IntegerField DB type.

This new type implements full unsigned u32 numbers by wrapping them around and storing them as signed i32 values in the DB.

In contrast to the existing BoundedPositiveIntegerField, this is not adding a DB CHECK constraint rejecting negative numbers. A migration was also added to change the various size and offset fields of the File/Blob/Index models to switch to this new type, which essentially drops the CHECK constraint from the DB, and thus allows negative DB values which will then be converted to a signed type on the client side.


Fixes https://github.com/getsentry/team-ingest/issues/690

@Swatinem Swatinem requested a review from Dav1dde April 7, 2025 10:09
@Swatinem Swatinem self-assigned this Apr 7, 2025
@Swatinem Swatinem requested a review from a team as a code owner April 7, 2025 10:09
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 7, 2025
@Swatinem Swatinem force-pushed the swatinem/4g-files branch from 90906a9 to c604e81 Compare April 7, 2025 10:13
Copy link
Contributor

github-actions bot commented Apr 7, 2025

This PR has a migration; here is the generated SQL for src/sentry/migrations/0864_file_offsets.py ()

--
-- Alter field size on controlfile
--
ALTER TABLE "sentry_controlfile" DROP CONSTRAINT "sentry_controlfile_size_check";
--
-- Alter field size on controlfileblob
--
ALTER TABLE "sentry_controlfileblob" DROP CONSTRAINT "sentry_controlfileblob_size_check";
--
-- Alter field offset on controlfileblobindex
--
ALTER TABLE "sentry_controlfileblobindex" DROP CONSTRAINT "sentry_controlfileblobindex_offset_check";
--
-- Alter field size on file
--
ALTER TABLE "sentry_file" DROP CONSTRAINT "sentry_file_size_check";
--
-- Alter field size on fileblob
--
ALTER TABLE "sentry_fileblob" DROP CONSTRAINT "sentry_fileblob_size_check";
--
-- Alter field offset on fileblobindex
--
ALTER TABLE "sentry_fileblobindex" DROP CONSTRAINT "sentry_fileblobindex_offset_check";

Copy link

codecov bot commented Apr 7, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
24964 1 24963 299
View the top 1 failed test(s) by shortest run time
tests.sentry.models.test_groupsubscription.GetParticipantsTest::test_does_not_include_nonmember
Stack Traces | 3.27s run time
#x1B[1m#x1B[.../sentry/models/test_groupsubscription.py#x1B[0m:795: in test_does_not_include_nonmember
    GroupSubscription.objects.filter(user_id=user.id, group=group).update(
#x1B[1m#x1B[.../sentry/silo/base.py#x1B[0m:158: in override
    return original_method(*args, **kwargs)
#x1B[1m#x1B[.../models/manager/base_query_set.py#x1B[0m:89: in update
    return super().update(**kwargs)
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:1253: in update
    rows = query.get_compiler(self.db).execute_sql(CURSOR)
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/sql/compiler.py#x1B[0m:2003: in execute_sql
    cursor = super().execute_sql(result_type)
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/sql/compiler.py#x1B[0m:1561: in execute_sql
    sql, params = self.as_sql()
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/sql/compiler.py#x1B[0m:1966: in as_sql
    val = field.get_db_prep_save(val, connection=self.connection)
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/fields/__init__.py#x1B[0m:1008: in get_db_prep_save
    return self.get_db_prep_value(value, connection=connection, prepared=False)
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/fields/__init__.py#x1B[0m:2130: in get_db_prep_value
    value = super().get_db_prep_value(value, connection, prepared)
#x1B[1m#x1B[31m.venv/lib/python3.13.../models/fields/__init__.py#x1B[0m:1001: in get_db_prep_value
    value = self.get_prep_value(value)
#x1B[1m#x1B[.../models/fields/bounded.py#x1B[0m:43: in get_prep_value
    assert self.MIN_VALUE <= value <= self.MAX_VALUE
#x1B[1m#x1B[31mE   AssertionError#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

This is achieved by introducing a new `WrappingU32IntegerField` DB type.

This new type implements full unsigned `u32` numbers by wrapping them around and storing them as signed `i32` values in the DB.

In contrast to the existing `BoundedPositiveIntegerField`, this is not adding a DB `CHECK` constraint rejecting negative numbers.
A migration was also added to change the various `size` and `offset` fields of the `File/Blob/Index` models to switch to this new type, which essentially drops the `CHECK` constraint from the DB, and thus allows negative DB values which will then be converted to a signed type on the client side.
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