-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(relocation) Use a shared bucket for relocations #116108
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
base: master
Are you sure you want to change the base?
Changes from 11 commits
c24ebe1
0362992
dd2dc81
be24349
17e6149
177a6e1
021d723
f3cae56
ec388cf
2975977
0ececbc
42b31cf
6764a8d
f2ad33b
93dc700
ba3a35b
8234909
144e5e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,6 @@ | |
| from sentry.api.exceptions import ResourceDoesNotExist | ||
| from sentry.api.permissions import SentryIsAuthenticated | ||
| from sentry.api.serializers import serialize | ||
| from sentry.models.files.file import File | ||
| from sentry.relocation.api.endpoints.index import ( | ||
| get_autopause_value, | ||
| validate_new_relocation_request, | ||
|
|
@@ -75,6 +74,7 @@ def post(self, request: Request, relocation_uuid: str) -> Response: | |
| status=status.HTTP_400_BAD_REQUEST, | ||
| ) | ||
|
|
||
| # TODO(cells) Update this when file becomes nullable/removed. | ||
| relocation_file = ( | ||
| RelocationFile.objects.filter(relocation=relocation).select_related("file").first() | ||
| ) | ||
|
|
@@ -85,10 +85,7 @@ def post(self, request: Request, relocation_uuid: str) -> Response: | |
| ) | ||
|
|
||
| # We can re-use the same `File` instance in the database, avoiding duplicating data. | ||
| try: | ||
| file = File.objects.get(id=relocation_file.file_id) | ||
| fileobj = file.getfile() | ||
| except (File.DoesNotExist, FileNotFoundError): | ||
| if not relocation_file.file: | ||
| return Response( | ||
| {"detail": ERR_FILE_NO_LONGER_EXISTS}, | ||
| status=status.HTTP_400_BAD_REQUEST, | ||
|
|
@@ -102,7 +99,7 @@ def post(self, request: Request, relocation_uuid: str) -> Response: | |
| ) | ||
|
|
||
| err = validate_new_relocation_request( | ||
| request, owner.username, relocation.want_org_slugs, fileobj.size | ||
| request, owner.username, relocation.want_org_slugs, relocation_file.file.size | ||
| ) or validate_relocation_uniqueness(owner) | ||
| if err is not None: | ||
| return err | ||
|
|
@@ -128,8 +125,9 @@ def post(self, request: Request, relocation_uuid: str) -> Response: | |
| ) | ||
| RelocationFile.objects.create( | ||
| relocation=new_relocation, | ||
| file=file, | ||
| kind=RelocationFile.Kind.RAW_USER_DATA.value, | ||
| bucket_path=relocation_file.bucket_path, | ||
| file=relocation_file.file, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Retry copies null bucket pathMedium Severity Failed-relocation retry reuses Additional Locations (1)Reviewed by Cursor Bugbot for commit 93dc700. Configure here. |
||
| ) | ||
|
|
||
| uploading_start.delay(str(new_relocation.uuid), None, None) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,15 +6,14 @@ | |
| import base64 | ||
| import logging | ||
| from datetime import UTC, datetime | ||
| from io import BytesIO | ||
| from uuid import UUID | ||
|
|
||
| from django.db import router | ||
| from django.db.utils import IntegrityError | ||
| from django.utils import timezone | ||
| from sentry_sdk import capture_exception | ||
|
|
||
| from sentry.models.files.file import File | ||
| from sentry.models.files.utils import get_relocation_storage | ||
| from sentry.relocation.models.relocation import Relocation, RelocationFile | ||
| from sentry.relocation.models.relocationtransfer import ( | ||
| RETRY_BACKOFF, | ||
|
|
@@ -27,7 +26,10 @@ | |
| ) | ||
| from sentry.relocation.tasks.process import fulfill_cross_region_export_request, uploading_complete | ||
| from sentry.relocation.tasks.transfer import process_relocation_transfer_control | ||
| from sentry.relocation.utils import RELOCATION_BLOB_SIZE, RELOCATION_FILE_TYPE | ||
| from sentry.relocation.utils import ( | ||
| RELOCATION_FILE_TYPE, | ||
| relocation_raw_data_path, | ||
| ) | ||
| from sentry.utils.db import atomic_transaction | ||
|
|
||
| logger = logging.getLogger("sentry.relocation") | ||
|
|
@@ -77,8 +79,7 @@ def reply_with_export( | |
| requesting_region_name: str, | ||
| replying_region_name: str, | ||
| org_slug: str, | ||
| encrypted_bytes: list[int], | ||
| # TODO(azaslavsky): finish transfer from `encrypted_contents` -> `encrypted_bytes`. | ||
| encrypted_bytes: list[int] | None = None, | ||
| encrypted_contents: bytes | None = None, | ||
| ) -> None: | ||
| with atomic_transaction( | ||
|
|
@@ -93,23 +94,22 @@ def reply_with_export( | |
| "requesting_region_name": requesting_region_name, | ||
| "replying_region_name": replying_region_name, | ||
| "org_slug": org_slug, | ||
| # TODO(azaslavsky): finish transfer from `encrypted_contents` -> `encrypted_bytes`. | ||
| "encrypted_bytes_size": len(encrypted_bytes or []), | ||
| } | ||
| logger.info("SaaS -> SaaS reply received in triggering region", extra=logger_data) | ||
| uuid = UUID(relocation_uuid) | ||
|
|
||
| try: | ||
| relocation: Relocation = Relocation.objects.get(uuid=relocation_uuid) | ||
| relocation: Relocation = Relocation.objects.get(uuid=uuid) | ||
| except Relocation.DoesNotExist as e: | ||
| logger.exception("Could not locate Relocation model by UUID: %s", relocation_uuid) | ||
| capture_exception(e) | ||
| return | ||
|
|
||
| # TODO(azaslavsky): finish transfer from `encrypted_contents` -> `encrypted_bytes`. | ||
| fp = BytesIO(bytes(encrypted_bytes or [])) | ||
| file = File.objects.create(name="raw-relocation-data.tar", type=RELOCATION_FILE_TYPE) | ||
| file.putfile(fp, blob_size=RELOCATION_BLOB_SIZE, logger=logger) | ||
| logger.info("SaaS -> SaaS relocation underlying File created", extra=logger_data) | ||
| # Now that we have confirmation that the export file | ||
| # was stored in the shared bucket, create a RelocationFile record | ||
| # so that the import process can begin. | ||
| # TODO(cells) Remove this once RelocationFile.file is optional. | ||
| file = File.objects.create(name="stub", type=RELOCATION_FILE_TYPE, size=10000) | ||
|
sentry[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| # This write ensures that the entire chain triggered by `uploading_start` remains | ||
| # idempotent, since only one (relocation_uuid, relocation_file_kind) pairing can exist | ||
|
Comment on lines
+114
to
120
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Catching an Suggested FixWhen an Prompt for AI Agent |
||
|
|
@@ -120,11 +120,11 @@ def reply_with_export( | |
| relocation=relocation, | ||
| file=file, | ||
| kind=RelocationFile.Kind.RAW_USER_DATA.value, | ||
| bucket_path=relocation_raw_data_path(uuid), | ||
| ) | ||
| except IntegrityError: | ||
| # We already have the file, we can proceed. | ||
| pass | ||
|
|
||
|
cursor[bot] marked this conversation as resolved.
|
||
| logger.info("SaaS -> SaaS relocation RelocationFile saved", extra=logger_data) | ||
|
|
||
| uploading_complete.apply_async(args=[relocation.uuid]) | ||
|
|
@@ -169,29 +169,17 @@ def reply_with_export( | |
| requesting_region_name: str, | ||
| replying_region_name: str, | ||
| org_slug: str, | ||
| encrypted_bytes: list[int], | ||
| # TODO(azaslavsky): finish transfer from `encrypted_contents` -> `encrypted_bytes`. | ||
| encrypted_bytes: list[int] | None = None, | ||
| encrypted_contents: bytes | None = None, | ||
| ) -> None: | ||
| logger_data = { | ||
| "uuid": relocation_uuid, | ||
| "requesting_region_name": requesting_region_name, | ||
| "replying_region_name": replying_region_name, | ||
| "org_slug": org_slug, | ||
| # TODO(azaslavsky): finish transfer from `encrypted_contents` -> `encrypted_bytes`. | ||
| "encrypted_bytes_size": len(encrypted_bytes or []), | ||
| } | ||
| logger.info("SaaS -> SaaS reply received on proxy", extra=logger_data) | ||
|
|
||
| # Save the payload into the control silo's "relocation" GCS bucket. This bucket is only used | ||
| # for temporary storage of `encrypted_bytes` being shuffled between cells like this. | ||
| path = f"runs/{relocation_uuid}/saas_to_saas_export/{org_slug}.tar" | ||
| relocation_storage = get_relocation_storage() | ||
| # TODO(azaslavsky): finish transfer from `encrypted_contents` -> `encrypted_bytes`. | ||
| fp = BytesIO(bytes(encrypted_bytes or [])) | ||
| relocation_storage.save(path, fp) | ||
| logger.info("SaaS -> SaaS export contents retrieved", extra=logger_data) | ||
|
|
||
| # Save transfer record so we can push state to the requesting cell | ||
| transfer = ControlRelocationTransfer.objects.create( | ||
| relocation_uuid=relocation_uuid, | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.