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 retry loop to deleting timeseries by name #6504

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

bnaecker
Copy link
Collaborator

@bnaecker bnaecker commented Sep 2, 2024

We currently support deleting timeseries by name in a schema upgrade. This deletion is implemented as a mutation in ClickHouse, which walks all affected data parts and deletes the relevant records in a merge operation. That's asynchronous by default, and run in a pool of background tasks. Despite that, with large tables, it can take a while for each mutation to complete, which blocks the server from queueing new deletion requests. This can lead to timeouts, like seen in #6501.

This should fix #6501, but I'm not certain of that because I don't have a good way to reproduce the bug. It seems likely that this is only seen when the database is already heavily loaded, as it might be when doing these mutations on large tables.

We currently support deleting timeseries by name in a schema upgrade.
This deletion is implemented as a mutation in ClickHouse, which walks
all affected data parts and deletes the relevant records in a merge
operation. That's asynchronous by default, and run in a pool of
background tasks. Despite that, with large tables, it can take a while
for each mutation to complete, which blocks the server from queueing new
deletion requests. This can lead to timeouts, like seen in #6501.

This should fix #6501, but I'm not certain of that because I don't have
a good way to reproduce the bug. It seems likely that this is only seen
when the database is already heavily loaded, as it might be when doing
these mutations on large tables.
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Looks good to me overall!

oximeter/db/src/client/mod.rs Outdated Show resolved Hide resolved
@bnaecker bnaecker enabled auto-merge (squash) September 9, 2024 17:08
@bnaecker bnaecker merged commit 63ec6f3 into main Sep 9, 2024
22 checks passed
@bnaecker bnaecker deleted the retry-timeseries-to-delete branch September 9, 2024 18:27
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.

Clickhouse-schema-updater failed to complete when going from version 5 to 11
2 participants