Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions src/sentry/runner/commands/cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None:
configure()

from sentry import deletions, models, similarity
from sentry.utils import metrics

skip_models = [
skip_child_relations_models = [
# Handled by other parts of cleanup
models.EventAttachment,
models.UserReport,
Expand All @@ -98,19 +99,25 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None:
return

model_name, chunk = j
Copy link
Member Author

Choose a reason for hiding this comment

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

This section is easier to review by hiding the white spaces:
https://github.com/getsentry/sentry/pull/101558/files?diff=split&w=1

model = import_string(model_name)

try:
task = deletions.get(
model=model,
query={"id__in": chunk},
skip_models=skip_models,
transaction_id=uuid4().hex,
)
with sentry_sdk.start_transaction(
op="cleanup", name="multiprocess_worker"
) as transaction:
transaction.set_tag("model", model_name)
model = import_string(model_name)
task = deletions.get(
model=model,
query={"id__in": chunk},
skip_models=skip_child_relations_models,
transaction_id=uuid4().hex,
)

while True:
if not task.chunk(apply_filter=True):
break
while True:
if not task.chunk(apply_filter=True):
break
except Exception:
metrics.incr("cleanup.error", instance=model_name, sample_rate=1.0)
logger.exception("Error in multiprocess_worker.")
finally:
task_queue.task_done()
Expand All @@ -137,7 +144,8 @@ def multiprocess_worker(task_queue: _WorkQueue) -> None:
"-t",
default=False,
is_flag=True,
help="Send the duration of this command to internal metrics.",
hidden=True,
help="(deprecated) Send the duration of this command to internal metrics.",
Copy link
Member Author

Choose a reason for hiding this comment

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

All scripts are passing the --timed parameter, thus, we don't need this.

)
@log_options()
def cleanup(
Expand Down Expand Up @@ -166,7 +174,6 @@ def cleanup(
concurrency=concurrency,
silent=silent,
router=router,
timed=timed,
)


Expand All @@ -178,7 +185,6 @@ def _cleanup(
concurrency: int,
silent: bool,
router: str | None,
timed: bool,
) -> None:
_validate_and_setup_environment(concurrency, silent)
# Make sure we fork off multiprocessing pool
Expand All @@ -189,18 +195,14 @@ def _cleanup(
# transaction context issues in child processes. This ensures only the
# main process tracks the overall cleanup operation performance.
with sentry_sdk.start_transaction(op="cleanup", name="cleanup") as transaction:
transaction.set_tag("router", router)
transaction.set_tag("model", model)
Copy link
Member Author

Choose a reason for hiding this comment

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

All errors are happening within the processes, thus, these taggings are useless.
I will remove the other one on a different commit/PR to save triggering CI.

try:
from sentry.runner import configure

configure()

from sentry.utils import metrics

start_time = None
if timed:
start_time = time.time()
start_time = time.time()

# list of models which this query is restricted to
model_list = {m.lower() for m in model}
Expand Down Expand Up @@ -286,7 +288,7 @@ def is_filtered(model: type[Model]) -> bool:
# Shut down our pool
_stop_pool(pool, task_queue)

if timed and start_time:
if start_time:
duration = int(time.time() - start_time)
metrics.timing("cleanup.duration", duration, instance=router, sample_rate=1.0)
click.echo("Clean up took %s second(s)." % duration)
Expand Down
Loading