Skip to content

Conversation

TartanLeGrand
Copy link
Contributor

This pull request introduces a new automated cleanup process for ClickHouse tables in Snuba, configurable via Helm chart values. The main changes add a CronJob resource for periodic cleanup, enable flexible table selection (auto-detection or fallback list), and provide configuration options for scheduling, retention, and resource management. It also adds a dedicated ServiceAccount for the cleanup job.

ClickHouse Cleanup CronJob Implementation:

  • Added a new Helm template cronjob-clickhouse-cleanup.yaml that creates a CronJob to periodically delete old data from ClickHouse tables based on a configurable retention period. The job supports auto-discovery of tables using include/exclude regex patterns, with a fallback hardcoded list if auto-detection fails.
  • Added configuration options under snuba.cleanup in values.yaml to control job scheduling, retention period, resource limits, table selection patterns, and other pod/job settings.

ServiceAccount and Permissions:

  • Added a new Helm template serviceaccount-clickhouse-cleanup.yaml to create a dedicated ServiceAccount for the cleanup job, with customizable annotations and token settings.

Cleanup and Refactoring:

  • Removed the unused and unimplemented symbolicator.cleanup section from values.yaml to avoid confusion and maintain chart clarity.

@Mokto Mokto merged commit e2e911d into sentry-kubernetes:develop Sep 15, 2025
2 checks passed
@Mokto
Copy link
Contributor

Mokto commented Sep 15, 2025

Really nice thanks!

@Mokto Mokto mentioned this pull request Sep 15, 2025
metadata:
annotations:
checksum/configYml: {{ .Values.config.configYml | toYaml | toString | sha256sum }}
checksum/config.yaml: {{ include "snuba.config" . | sha256sum }}
Copy link
Contributor

Choose a reason for hiding this comment

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

@Mokto @TartanLeGrand This should be "sentry.snuba.config"

@manishrawat1992
Copy link

@TartanLeGrand why did we merge this. These tables are supposed to autocleanup by their configured retention policy.

@TartanLeGrand
Copy link
Contributor Author

@TartanLeGrand why did we merge this. These tables are supposed to autocleanup by their configured retention policy.

getsentry/self-hosted#1808 (comment)

@cjcormack
Copy link
Contributor

@TartanLeGrand I think I am missing something while looking at that referenced issue. From what I can see, Snuba is respecting our retention policy (oldest record I can see in CH is 90 days old). While this change is probably useful, I think enabling it by default with a shorter retention period than the built-in cleanup (sentry.cleanup.days defaults to 90) is going to catch people out.

@manishrawat1992
Copy link

manishrawat1992 commented Sep 16, 2025

@TartanLeGrand why did we merge this. These tables are supposed to autocleanup by their configured retention policy.

getsentry/self-hosted#1808 (comment)

No, that is node store which is in postgres not in snuba.
for time being I will keep snuba.cleanup.enabled: false

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants