Skip to content

Fix NOT_ENOUGH_SPACE error from CH during monthly clean sites job #6551

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.

Sign up for GitHub

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

Open
apata wants to merge 2 commits into
base: master
Choose a base branch
from
+0 −152
Open

Fix NOT_ENOUGH_SPACE error from CH during monthly clean sites job #6551

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84ca882
Fix NOT_ENOUGH_SPACE error from CH during monthly clean sites job
apata Jul 27, 2026
fd76b38
Remove job entirely
apata Jul 30, 2026
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
81 changes: 70 additions & 11 deletions lib/workers/clickhouse_clean_sites.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
defmodule Plausible.Workers.ClickhouseCleanSites do
@moduledoc """
Cleans deleted site data from ClickHouse asynchronously.
Cleans deleted site data from ClickHouse sequentially.

We batch up data deletions from ClickHouse as deleting a single site is
just as expensive as deleting many.
We batch up data deletions from ClickHouse as deleting a single site may be more expensive than deleting many:
the expense scales with the number of partitions the site or sites have rows in.
A single site with events since 2025-01 to 2026-06 will be more expensive to clean up
than a hundred sites with events only in the partition 2026-06.

Tables are cleaned one partition at a time because to clean one partition,
Clickhouse needs to rewrite it. It reserves room for rewriting it in full on disk.
Cleaning all partitions at the same time would mean Clickhouse reserving room on disk
equal to the size of all the partitions,
potentially reserving all the available disk space and shorting out INSERTs from ingestion.
This sequential approach prevents that.
"""

use Plausible.Repo
Expand Down Expand Up @@ -31,7 +40,9 @@ defmodule Plausible.Workers.ClickhouseCleanSites do
"imported_visitors"
]

@settings if Mix.env() in [:test, :ce_test, :e2e_test], do: [mutations_sync: 2], else: []
@settings [mutations_sync: 2]

@aerosol aerosol Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pretty sure this should be async on prod

@apata apata Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is a bit of a step away from tradition and you're right to point this out. If mutations_sync: 2, we wait for all nodes in the cluster to finish the mutation, which is a bit flimsy if one of the nodes is in trouble at the time.

If mutations_sync: 0, we dump the mutations into a list on CH cluster Keeper, hang up and the job completes with :ok. CH then churns through these as and when able (https://clickhouse.com/docs/reference/statements/alter/index#mutations). The mutations may succeed or they may fail. How do we know that we actually cleaned up that which we intended? There's also the matter of mutation concurrency. With this method, we lose control over it (beyond settings levers), which can lead to mutations using up all the available disk space.

I'm ok with mutations_sync: 0, but in this case we should run another job soon after to check up on these mutations.

If mutations are absolutely necessary, monitor them carefully using the system.mutations table and use KILL MUTATION if a process is stuck or misbehaving. Misusing mutations can lead to degraded performance, excessive storage churn, and potential service instability—so apply them with caution and sparingly.

(from https://clickhouse.com/docs/concepts/best-practices/avoid-mutations)

@aerosol aerosol Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think running a single oban job potentially for hours/days is unacceptable design-wise. If you think you can recover from a synchronous timeout or other failure by retrying the job, please make it so each partition prune is a job on its own and oban takes care of scheduling.

@aerosol aerosol Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The mutations may succeed or they may fail. How do we know that we actually cleaned up that which we intended?

Why do you need to know right away? What is the consequence of one partition not getting pruned because site id 2387 was deleted? On a scale of no one is affected to catastrophic, what is the severity?

Monitoring ClickHouse for its inability to execute mutations consistently is another matter, completely out of scope, so we're talking a one-off event where a partition isn't fully rewritten. What is the big deal?

@apata apata Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Monitoring ClickHouse for its inability to execute mutations consistently is another matter, completely out of
scope, so we're talking a one-off event where a partition isn't fully rewritten. What is the big deal?

With the currently deployed method that's using mutations_sync: 0, if the mutation cleaning up events_v2 table for a batch of sites succeeded, but cleaning up other tables failed, when will the data for those sites be cleaned up from the other tables?

Data deletion is a critical part of security and privacy guarantees to users and I look at it with utmost severity, so I don't think monitoring mutations we rely on to ensure this is out of scope. CH docs explicitly advise monitoring mutations. Still, it can be done over our monitoring platform and with alerts, not in app code as I suggested.

I think running a single oban job potentially for hours/days is unacceptable design-wise.

Agreed, converting to draft and will discuss alternatives, separate job per partition per table or similar.

@aerosol aerosol Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Data deletion is a critical part of security and privacy guarantees to users and I look at it with utmost severity

Care to elaborate how the data we store at rest is not private and how this is important for security? The site ID doesn't exist anymore, there's a bunch of anonymous visits with meaningless identifiers worst case, on a rare event (we have no record of) of a partial mutation failing.

so I don't think monitoring mutations we rely on to ensure this is out of scope

Out of scope of ensuring ingestion is uninterrupted due to deletions happening

@apata apata Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The data about website visits is valuable, as evidenced by analytics platforms that offer their service for free in exchange for some access to the data.

This value and meaning persists even if the site_id that it is for doesn't exist any more. In events_v2 we have hostname, page, meta, ...

Failure to clean up this properly means that in the event of a data leak, data of website visitors of sites that have stopped using Plausible would be exposed as well.

@aerosol aerosol Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the event of data leak, the data we failed to delete is our least concern. What would need to happen so that we'd leak partially deleted data but not everything else? Food for thought.


@partition_delete_timeout :timer.minutes(15)

@aerosol aerosol Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So are we ok to crash if this takes more than 15 mins? What's the reasoning here?

@apata apata Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Important question, we must take fails / timeouts into account when working on another solution.


def perform(_job) do
deleted_sites = get_deleted_sites_with_clickhouse_data()

@aerosol aerosol Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if you like to do that, but we could store the deleted IDs upon deletion and pick them up from there instead of dancing around with mapset and somewhat greedy queries. We could run this exact cleanup for the last time manually and keep reading exactly what we need from a trusted source.

@apata apata Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea. These queries are scanning a lot of rows. We will need to discuss how to implement the removal of a site_id from that table: it must not happen before all mutations cleaning the data for the site for all tables for all partitions have run, but it's not obvious to me where we can get that signal.

Expand All @@ -41,18 +52,59 @@ defmodule Plausible.Workers.ClickhouseCleanSites do
"Clearing ClickHouse data for the following #{length(deleted_sites)} sites which have been deleted: #{inspect(deleted_sites)}"
)

database = current_database()

for table <- @tables_to_clear do
IngestRepo.query!(
"ALTER TABLE {$0:Identifier} DELETE WHERE site_id IN {$1:Array(UInt64)}",
[table, deleted_sites],
settings: @settings
)
clean_sites_from_table(database, table, deleted_sites)
end
end

:ok
end

defp clean_sites_from_table(database, table, deleted_sites_ids) do
for partition_id <- active_partition_ids(database, table) do
delete_sites_from_partition(table, partition_id, deleted_sites_ids)
end
end

defp delete_sites_from_partition(table, partition_id, deleted_sites_ids) do
IngestRepo.query!(
"ALTER TABLE {$0:Identifier} DELETE IN PARTITION ID {$1:String} WHERE site_id IN {$2:Array(UInt64)}",
[table, partition_id, deleted_sites_ids],
settings: @settings,
timeout: @partition_delete_timeout,
checkout_retries: 0

@aerosol aerosol Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why checkout_retries: 0? Are we effectively rescheduling this partition for next month if the pool is busy?

@apata apata Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid question! Need to discuss when working on another solution

@aerosol aerosol Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, but why did you write it even? 😅

@apata apata Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I appreciate your input and wanted to clarify that it will not be ignored, just that I didn't want to go in depth for a solution that is inadequate for other reasons.

Since timeouts and failures are such an integral part of any new solution, I'd prefer to discuss it with the proposal of the new system at hand.

These comments are a public record of how things are done at Plausible, so I thought it's appropriate to take an extra minute to ack.

@apata apata Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because this may seem as an evasion of responsibility on my part, I want to clarify that I removed the retries intentionally, without realising that retrying won’t make or break this job: it doesn’t matter whether there’ll be 1x or 4x attempts for each partition when the count of partitions this job would have needed to sequentially process without errors is 1000+. They’re pretty chunky partitions as well, so we need to find another way.

)
end

defp active_partition_ids(database, table) do
source =
if IngestRepo.clustered_table?(table) do
"clusterAllReplicas('{cluster}', system.parts)"
else
"system.parts"
end

%Ch.Result{columns: ["partition_id"], rows: rows} =
IngestRepo.query!(
"""
SELECT DISTINCT partition_id
FROM #{source}
WHERE database = {$0:String} AND table = {$1:String} AND active
ORDER BY partition_id
""",
[database, table]
)

Enum.map(rows, fn [partition_id] -> partition_id end)

@aerosol aerosol Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Enum.map(rows, fn [partition_id] -> partition_id end)
List.flatten(rows)

end

defp current_database do
%Ch.Result{rows: [[database]]} = IngestRepo.query!("SELECT currentDatabase()")
database
end

def get_deleted_sites_with_clickhouse_data() do
pg_sites =
from(s in Plausible.Site.regular(), select: s.id)
Expand All @@ -67,15 +119,22 @@ defmodule Plausible.Workers.ClickhouseCleanSites do
DBConnection.run(
ch,
fn conn ->
Ch.query!(conn, "FROM events_v2 SELECT site_id GROUP BY site_id", [],
Ch.query!(conn, site_ids_with_data_query(), [],
settings: [optimize_distinct_in_order: 1],
timeout: :infinity
)
end,
timeout: :infinity
)

ch_sites = rows |> MapSet.new(fn [site_id] -> site_id end)
ch_sites = for [site_id] <- rows, not is_nil(site_id), into: MapSet.new(), do: site_id

MapSet.difference(ch_sites, pg_sites) |> MapSet.to_list()
end

defp site_ids_with_data_query do
Enum.map_join(@tables_to_clear, " UNION DISTINCT ", fn table ->
"SELECT DISTINCT site_id FROM #{table}"
end)
end
end
66 changes: 60 additions & 6 deletions test/workers/clickhouse_clean_sites_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,25 @@ defmodule Plausible.Workers.ClickhouseCleanSitesTest do
deleted_site = insert(:site)

populate_stats(site, [
build(:pageview)
build(:pageview),
build(:pageview, timestamp: ~D[2026-01-01]),
build(:imported_visitors),
build(:imported_sources),
build(:imported_pages),
build(:imported_entry_pages),
build(:imported_exit_pages),
build(:imported_locations),
build(:imported_devices),
build(:imported_browsers),
build(:imported_operating_systems),
build(:imported_custom_events)
])

populate_stats(deleted_site, [
build(:pageview),
build(:pageview),
build(:pageview, timestamp: ~D[2026-01-01]),
build(:pageview, timestamp: ~D[2026-02-01]),
build(:pageview, timestamp: ~D[2026-03-01]),
build(:imported_visitors),
build(:imported_sources),
build(:imported_pages),
Expand All @@ -24,7 +37,8 @@ defmodule Plausible.Workers.ClickhouseCleanSitesTest do
build(:imported_locations),
build(:imported_devices),
build(:imported_browsers),
build(:imported_operating_systems)
build(:imported_operating_systems),
build(:imported_custom_events)
])

Repo.delete!(deleted_site)
Expand Down Expand Up @@ -52,17 +66,57 @@ defmodule Plausible.Workers.ClickhouseCleanSitesTest do
assert_count(deleted_site, "imported_devices", 0)
assert_count(deleted_site, "imported_browsers", 0)
assert_count(deleted_site, "imported_operating_systems", 0)
assert_count(site, "events_v2", 1)
assert_count(site, "sessions_v2", 1)
assert_count(deleted_site, "imported_custom_events", 0)
assert_count(site, "events_v2", 2)
assert_count(site, "sessions_v2", 2)
assert_count(site, "imported_visitors", 1)
assert_count(site, "imported_sources", 1)
assert_count(site, "imported_pages", 1)
assert_count(site, "imported_entry_pages", 1)
assert_count(site, "imported_exit_pages", 1)
assert_count(site, "imported_locations", 1)
assert_count(site, "imported_devices", 1)
assert_count(site, "imported_browsers", 1)
assert_count(site, "imported_operating_systems", 1)
assert_count(site, "imported_custom_events", 1)

assert not Enum.member?(
ClickhouseCleanSites.get_deleted_sites_with_clickhouse_data(),
deleted_site.id
)
end

@tag :slow
test "cleans a deleted site that has only imported data and no native events" do
kept_site = insert(:site)
imported_only = insert(:site)

# A live site with native events - must be left untouched.
populate_stats(kept_site, [build(:pageview)])

populate_stats(imported_only, [
build(:imported_visitors),
build(:imported_sources),
build(:imported_custom_events)
])

Repo.delete!(imported_only)

assert Enum.member?(
ClickhouseCleanSites.get_deleted_sites_with_clickhouse_data(),
imported_only.id
)

ClickhouseCleanSites.perform(nil)

assert_count(imported_only, "imported_visitors", 0)
assert_count(imported_only, "imported_sources", 0)
assert_count(imported_only, "imported_custom_events", 0)
assert_count(kept_site, "events_v2", 1)
end

def assert_count(site, table, expected_count) do
q = from(e in table, select: %{count: fragment("count()")}, where: e.site_id == ^site.id)
await_clickhouse_count(q, expected_count)
assert await_clickhouse_count(q, expected_count)
end
end
Loading