From 204ccd51b4f276e3bec98bc9f62968f1b5260164 Mon Sep 17 00:00:00 2001 From: Alex Zaslavsky Date: Tue, 2 Apr 2024 16:11:05 -0700 Subject: [PATCH] fix(relocation): Right size validation step timeouts These had previously been increased in https://github.com/getsentry/sentry/pull/68080 and again in https://github.com/getsentry/sentry/pull/68120 to aid with debugging. Now that https://github.com/getsentry/sentry/pull/68140 has addressed the root cause of these excessively long imports, we can bring these timeouts down to a more reasonable level. One benefit of the episode is that we now know how long it takes to import/export a ~50MB relocation file. Based on this, we've set the timeouts back to 5 minutes for tasks we expect to be quick (handling `baseline-config` and `colliding-user` steps), while increasing the timeouts for the actual relocation import/export steps to 40 minutes. Since even very large comparisons proved quite quick, we're lowering that timeout to 2 minutes. --- src/sentry/utils/relocation.py | 16 +++++++++++++--- .../test_success.pysnap | 12 ++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/sentry/utils/relocation.py b/src/sentry/utils/relocation.py index 002affb436cc88..e10582591c9daa 100644 --- a/src/sentry/utils/relocation.py +++ b/src/sentry/utils/relocation.py @@ -219,7 +219,7 @@ class OrderedTask(Enum): - "--findings-file" - "/findings/import-$jsonfile" $args - timeout: 2400s + timeout: $timeout """ ) @@ -250,7 +250,7 @@ class OrderedTask(Enum): - "--findings-file" - "/findings/export-$jsonfile" $args - timeout: 2400s + timeout: $timeout """ ) @@ -297,7 +297,7 @@ class OrderedTask(Enum): - "--findings-file" - "/findings/compare-$jsonfile" $args - timeout: 300s + timeout: $timeout """ ) @@ -574,6 +574,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="import-baseline-config", step=IMPORT_VALIDATION_STEP_TEMPLATE, scope="config", + timeout=300, wait_for=[], kind=RelocationFile.Kind.BASELINE_CONFIG_VALIDATION_DATA, args=["--overwrite-configs"], @@ -582,6 +583,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="import-colliding-users", step=IMPORT_VALIDATION_STEP_TEMPLATE, scope="users", + timeout=300, wait_for=["import-baseline-config"], kind=RelocationFile.Kind.COLLIDING_USERS_VALIDATION_DATA, args=filter_usernames_args, @@ -590,6 +592,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="import-raw-relocation-data", step=IMPORT_VALIDATION_STEP_TEMPLATE, scope="organizations", + timeout=2400, wait_for=["import-colliding-users"], kind=RelocationFile.Kind.RAW_USER_DATA, args=filter_org_slugs_args, @@ -598,6 +601,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="export-baseline-config", step=EXPORT_VALIDATION_STEP_TEMPLATE, scope="config", + timeout=300, wait_for=["import-raw-relocation-data"], kind=RelocationFile.Kind.BASELINE_CONFIG_VALIDATION_DATA, args=[], @@ -606,6 +610,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="export-colliding-users", step=EXPORT_VALIDATION_STEP_TEMPLATE, scope="users", + timeout=300, wait_for=["export-baseline-config"], kind=RelocationFile.Kind.COLLIDING_USERS_VALIDATION_DATA, args=filter_usernames_args, @@ -614,6 +619,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="export-raw-relocation-data", step=EXPORT_VALIDATION_STEP_TEMPLATE, scope="organizations", + timeout=2400, wait_for=["export-colliding-users"], kind=RelocationFile.Kind.RAW_USER_DATA, args=filter_org_slugs_args, @@ -627,6 +633,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="compare-baseline-config", step=COMPARE_VALIDATION_STEP_TEMPLATE, scope="config", + timeout=120, wait_for=["export-raw-relocation-data"], kind=RelocationFile.Kind.BASELINE_CONFIG_VALIDATION_DATA, args=[], @@ -635,6 +642,7 @@ def create_cloudbuild_yaml(relocation: Relocation) -> bytes: id="compare-colliding-users", step=COMPARE_VALIDATION_STEP_TEMPLATE, scope="users", + timeout=120, wait_for=["compare-baseline-config"], kind=RelocationFile.Kind.COLLIDING_USERS_VALIDATION_DATA, args=[], @@ -664,6 +672,7 @@ def create_cloudbuild_validation_step( scope: str, wait_for: list[str], kind: RelocationFile.Kind, + timeout: int, args: list[str], ) -> str: return step.substitute( @@ -674,5 +683,6 @@ def create_cloudbuild_validation_step( kind=str(kind), scope=scope, tarfile=kind.to_filename("tar"), + timeout=str(timeout) + "s", wait_for=make_cloudbuild_step_args(3, wait_for), ) diff --git a/tests/sentry/tasks/snapshots/PreprocessingTransferTest/test_success.pysnap b/tests/sentry/tasks/snapshots/PreprocessingTransferTest/test_success.pysnap index 04f4dcbc5fbb15..f0f59147d3f252 100644 --- a/tests/sentry/tasks/snapshots/PreprocessingTransferTest/test_success.pysnap +++ b/tests/sentry/tasks/snapshots/PreprocessingTransferTest/test_success.pysnap @@ -127,7 +127,7 @@ steps: - --overwrite-configs id: import-baseline-config name: gcr.io/cloud-builders/docker - timeout: 2400s + timeout: 300s waitFor: - copy-inputs-being-validated - create-working-dirs @@ -157,7 +157,7 @@ steps: - importing id: import-colliding-users name: gcr.io/cloud-builders/docker - timeout: 2400s + timeout: 300s waitFor: - copy-inputs-being-validated - create-working-dirs @@ -221,7 +221,7 @@ steps: - /findings/export-baseline-config.json id: export-baseline-config name: gcr.io/cloud-builders/docker - timeout: 2400s + timeout: 300s waitFor: - import-baseline-config - import-raw-relocation-data @@ -254,7 +254,7 @@ steps: - importing id: export-colliding-users name: gcr.io/cloud-builders/docker - timeout: 2400s + timeout: 300s waitFor: - import-colliding-users - export-baseline-config @@ -329,7 +329,7 @@ steps: - /findings/compare-baseline-config.json id: compare-baseline-config name: gcr.io/cloud-builders/docker - timeout: 300s + timeout: 120s waitFor: - export-baseline-config - export-raw-relocation-data @@ -361,7 +361,7 @@ steps: - /findings/compare-colliding-users.json id: compare-colliding-users name: gcr.io/cloud-builders/docker - timeout: 300s + timeout: 120s waitFor: - export-colliding-users - compare-baseline-config