Skip to content

Reduce site deletions impact on ClickHouse - #6591

Open
aerosol wants to merge 30 commits into
masterfrom
delete-site-improve
Open

Reduce site deletions impact on ClickHouse #6591
aerosol wants to merge 30 commits into
masterfrom
delete-site-improve

Conversation

@aerosol

@aerosol aerosol commented Aug 11, 2026

Copy link
Copy Markdown
Member

Changes

Remix of #6551

https://3.basecamp.com/5308029/buckets/36789884/card_tables/cards/10064243324

For reviewers: commit order matters, reading the messages alone should somewhat reflect the thought process. Migration will be extracted to a separate PR once review is concluded.

This PR changes how periodic site cleanup works:

  • Pending deletions are stored for background execution as a part of the Postgres site deletion transaction
  • Pending deletions are cleaned up once handled by the (now) weekly worker
  • Pending deletions store imported/pageview ranges per site, so that data boundary is kept
  • Site.Cache entry eviction is broadcasted upon Postgres site deletion so we stop accepting traffic ASAP. Edge case: it is still possible we'll keep some sessions_v2 around if they span over dates (sessions_v2 timestamp vs start). Classifying this as negligible for now.
  • Lightweight deletions are issued where possible
  • Dedicated ClickHouse connection is used for deletions (the same that we used to use for user-initiated import cancellations). Its connection pool was bumped from 1 to 2, just in case. This is to avoid extra ingest interference.
  • Previously used orphaned site IDs listing is kept as a function, to be executed on prod once for the last time (I thought data migration would've been an overkill). The function effectively back fills pending deletions.
  • Even though most of worker operation is asynchronous, basic worker metrics are exposed to the scraper. Again, considered polling ongoing mutations metrics, but there is no good way of monitoring/identifying the mutations only this part of app might've triggered, so leaving ClickHouse monitoring out of scope (for now at least).
  • Pending deletions table is somewhat extensible - we list deletions by reason, so distinct ones could be implemented later on.
  • For partitioned tables, deletions are enqueued partition by partition - we are operating under the assumption that the app isn't the best place to manage ClickHouse's internal queue.

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

Comment thread lib/plausible/stats/clickhouse.ex Outdated
Comment thread lib/workers/clickhouse_clean_sites.ex
Comment thread priv/repo/migrations/20260810060815_pending_stats_deletions.exs Outdated
Comment thread lib/plausible/stats/clickhouse.ex Outdated
Comment thread lib/plausible/pending_stats_deletions.ex Outdated
Comment thread lib/plausible/site/cache.ex

@ukutaht ukutaht left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inline comments aside, I have some reservations about the overall direction in this PR.

It introduces a worklist (PendingSiteDeletions) over calculating the set of site ids to be deleted by comparing ids in postgres vs clickhouse as was done previously. With this design I'm seeing new failure modes and edge cases that did not exist before. There's a potential timing issue with cache eviction. It also creates a synchronization point where site deletions must always add a deletion entry as well. Baking date ranges from postgres into it causes pretty significant errors.

The benefit of the previous design was that however sites were deleted, the ClickhouseCleanSites worker would always move towards a stable state where the set of site ids present in clickhouse is equal to the ones in postgres. The new design does not have that property and introduces the possibility (or even likelihood) that the two sets will diverge over time.

The way I see it, the proposed design loses some great properties we used to get for free. The previous contract of "when site is deleted from postgres, it will eventually be sweeped from clickhouse" was better decoupled from the rest of the application, simple to reason about and hard to get wrong.

What's the reasoning for adding the PendingSiteDeletions table? What problem is it solving that the previous design didn't?

The core issue with the old design was execution strategy that ended in big and heavy mutations. My current view is that it would be better to keep the old design for deciding which sites to delete, and only touch the execution part of it.

Notes about execution strategy

This PR adds partitioning and lightweight deletes. Running a local test for how much space the mutations claim, the result is the following:

One table, 3 monthly partitions, 9M rows, 74.7 MiB on disk. Deleting one site out of 5,000 — 0.02% of rows:

┌─────────────────────────────────────────┬───────────────────┬───────────────────────────────────┐
│                strategy                 │ extra disk needed │           as % of table           │
├─────────────────────────────────────────┼───────────────────┼───────────────────────────────────┤
│ ALTER … DELETE WHERE (current)          │ 74.8 MiB          │ 100.2% — whole table duplicated   │
├─────────────────────────────────────────┼───────────────────┼───────────────────────────────────┤
│ ALTER … DELETE IN PARTITION ID '202502' │ 25.0 MiB          │ 33.4% — exactly 1 of 3 partitions │
├─────────────────────────────────────────┼───────────────────┼───────────────────────────────────┤
│ DELETE FROM (lightweight)               │ 73.5 KiB          │ 0.096%                            │
└─────────────────────────────────────────┴───────────────────┴───────────────────────────────────┘

This shows the problem with the current strategy - the whole table needs to be copied to run the mutation. On the other hand, lightweight deletes use negligible disk space. It seems like we should be able to issue a whole-table lightweight delete with no worries about disk space.

Running heavy mutations in a partitioned manner is also an option since it does reduce disk space needed meaningfully and would fix the prod issue.

But perhaps the simplest thing to try is to switch the current worker to use lightweight deletes without changing anything else and seeing how it works. Given that it comes with a risk of breaking ingestion it's probably worth verifying in some capacity on prod as well before enabling.

@apata got a different result here. Not sure what the truth is on lightweight vs heavy deletes and the need to partition them.


{:ok, pending_stats_deletion} = PendingStatsDeletions.store(site)

result = Repo.delete_all(from(s in Plausible.Site, where: s.domain == ^site.domain))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since PendingStatsDeletions is keyed on site.id rather than site.domain, it's also worth changing this line to delete via the same site.id.

There's a small chance with some domain change shenanigans that things will get weird otherwise.


@spec store(Site.t(), atom()) :: {:ok, PendingStatsDeletion.t() | nil}
def store(%Site{} = site, reason \\ :user_request) do
case Sites.stats_range(site) do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does not account for sites that have reset their stats. When stats are reset, the site.native_stats_start_at is bumped while leaving Clickhouse rows untouched. The result is that when we clean data for a site like this, the removal process will also leave those stats untouched.

To properly establish a date range to clean I think it needs to come from Clickhouse rather than relying on postgres pointers which are not guaranteed to match Clickhouse.

@ukutaht ukutaht Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Further explanation: assume a site whose clickhouse native data starts from 2026-01-01 and the native_stats_start_date is also at 2026-01-01. No imports. Now take the following steps:

  1. Reset stats today (Plausible.Purge.reset!/1 bumps Site.native_stats_start_date to 2026-08-12)
  2. Delete the site
  3. Expected result: PendingStatsDeletion is inserted with stats_start_date: 2026-01-01
  4. Actual result: depends on whether any events were ingested after stats were reset
    4a. If no events received: no PendingStatsDeletion inserted at all because Plausible.Sites.stats_range/1 returns {nil, nil}
    4b. If some events received: PendingStatsDeletion is inserted with stats_start_date being the date of the first event received after 2026-08-12

Fundamentally the issue is that Plausible.Stats.Clickhouse.pageview_start_date_local/2 uses site.native_stats_start_date as a filter so the start date is clamped to the postgres field. But this postgres field is not a faithful representation of when clickhouse data started. It can be moved by the reset operation Plausible.Purge.reset/1 and it can also be overwritten in the CRM. There is no postgres field that tracks the actual start date of native clickhouse stats.

p.site_id,
p.site_id
),
stats_start: min(p.stats_start_date),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This value propagates to Clickhouse.partition_ids to choose the range of partitions to touch. This value also contains imported data whose range is essentially user input and can go very far back.

So if a batch contains imports from 2005, we issue 264 mutations over 132 partitions for native tables that for most of that range have no data at all. They will essentially be no-ops but not free, Clickhouse will still process them as mutations. Better to avoid.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Essentially it's another point towards not using postgres pointers for deriving partition ranges. The postgres pointer includes imported data and forgets about resets so it's not a reliably indicator for what needs to be deleted.

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 partition_ids could be simplified by querying Clickhouse for the partitions

SELECT DISTINCT _partition_id FROM events_v2 WHERE site_id IN (…)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aerosol Explaining further:

Assume a site with native data starting on 2026-01-01 and an import with start_date on 2005-01-01.
When this site is deleted, a PendingStatsDeletion entry is added with stats_start_date=2005-01-01

When ClickhouseCleanSites requests a batch of pending deletions with this entry included, the stats_start_date for the whole batch becomes 2005-01-01. This is passed as the start date to Clickhouse.partition_ids which generates partition IDs all the way back to 2005, most of which do not exist in our partitioned native tables. Result is that mutations are issued for non-existent partitions in events_v2 and sessions_v2 tables.

I'm not sure what the cost of issuing no-op mutations is at the database level. Just flagging that a single site with a long import history would affect the whole batch like this.

SELECT DISTINCT _partition_id FROM events_v2 WHERE site_id IN (…)

This feels a lot better. Perhaps it would also enable removing stats_start_date and stats_end_date from PendingStatsDeletion entry altogether and also fix the reset stats issue with start date calculation.

Comment thread lib/plausible/site/cache.ex
PlausibleMetrics.measure_duration(telemetry_stage_duration(), fun, %{stage: stage})
end

defp clear_partitioned_table!(table, partition_id, site_ids) do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps I don't understand the characteristics between Heavy vs Lightweight deletes enough but this is surprising: I thought the benefit of lightweight deletes was that we don't need app-side partitioning because these are lighter. And @cnkk can apply the masks at in a partitioned fashion at a convenient time.

If we do need to orchestrate partitioning app-side anyways, is there any benefit to the two-step process of lightweight deletes?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My understanding is without IN PARTITION ID, a mutation (lightweight or not) still gets scheduled against every existing part across the entire table's history. The db can skip non-matching parts via primary key, but it still has to open and check each. Idk, but keeping it partitioned doesn't seem harmful - mutations are kept small and independently executable. Perhaps that's better for ClickHouse's internal queue and monitoring as opposed to one big hog? I am totally guessing though.

Repo.insert(%PendingStatsDeletion{
site_id: site.id,
stats_start_date: stats_start_date,
stats_end_date: stats_end_date,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The end date gets fixed when PendingStatsDeletion record is created but Site.Cache eviction is best effort and async

There is a potential timing bug here where PendingStatsDeletion is run close to a month boundary so some remaining events get added to the partition after stats_end_date. It would result in some (albeit small amount) of orphaned events.

@aerosol

aerosol commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Some good points, especially the stats dates quirk that you both noticed.
I failed to see the "reset stats" scenario and the fact that we should not estimate partition range based on imported data.

With regards to partitioning lightweight deletes, my understanding was it's always helpful to chunk work, especially given that ClickHouse was running mutations (on 202507) for a partitioned lightweight delete in a test @apata conducted:

image

I also started thinking of ClickHouse queue as of configurable self-managing capacity rather than limitation.

Anyway:

What's the reasoning for adding the PendingSiteDeletions table? What problem is it solving that the previous design didn't?

The idea was exactly to track sites that need to be deleted, instead of running quite expensive queries that may get even more expensive over the coming years.
In my mind this had the extra benefit of not doing any work at all, if there's nothing to delete.

Moreover I thought of reusing that table for handling expired trials/abandoned accounts ahead of time (different reason). Querying it on a weekly basis could be helpful for monitoring/reporting insight.

But perhaps the simplest thing to try is to switch the current worker to use lightweight deletes without changing anything else and seeing how it works.

I like it, we can try that, but:

The benefit of the previous design was that however sites were deleted, the ClickhouseCleanSites worker would always move towards a stable state where the set of site ids present in clickhouse is equal to the ones in postgres.

It sounds nice on paper but the previous design wasn't querying sessions_v2, just events_v2 to determine the set of site IDs, which was never stable state - there were some dangling entries left in the former table. And per @apata's finding, imported tables also contain IDs that don't exists anywhere else.

The heavy querying I mentioned becomes heavier if we want to address that AND keep the old way of determining the IDs that need to be removed.

What direction would you like see here?

Given that it comes with a risk of breaking ingestion it's probably worth verifying in some capacity on prod as well before enabling.

What do you mean by "risk of breaking ingestion" here? The fact that current clean up worker uses IngestRepo pool or are you suggesting lightweight deletes alone will affect ingestion somehow?

DeletionRepo.query!(
"DELETE FROM {$0:Identifier} IN PARTITION ID {$1:String} WHERE site_id IN {$2:Array(UInt64)}",
[table, partition_id, site_ids],
settings: @settings

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.

we need lightweight_deletes_sync = 0 if we want it run async or at least = 1 to wait for the current node.

p.site_id,
p.site_id
),
stats_start: min(p.stats_start_date),

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 partition_ids could be simplified by querying Clickhouse for the partitions

SELECT DISTINCT _partition_id FROM events_v2 WHERE site_id IN (…)

@ukutaht

ukutaht commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

@aerosol Thanks. Getting to the heart of the discussion on the PendingStatsDeletions design

The idea was exactly to track sites that need to be deleted, instead of running quite expensive queries that may get even more expensive over the coming years.

Right. I hadn't really considered this as an issue because we haven't hit any limits on this as far as I can tell. Artur said the clickhouse query currently takes 20 seconds. In the medium term I would expect queries like this to get less expensive. As we start clearing inactive customers from the database, the dataset to query each time will be smaller since we have a long history of inactive customers still in both databases. But I see the long-term concern.

On one hand I agree - a 20 second query on clickhouse side feels a bit uncomfortable. Same for reading all site ids (six figure number) from postgres to app memory. On the other hand I don't think these have hit any limits yet so I wonder if it's a 'yagni' type of thing. I'm conflicted about whether the site id querying in the old design is a problem or not without knowing where the actual limits are.

I can definitely see the benefit in the worklist design in that the query heaviness and large site_id sets being shuffled around is not something we need to worry about in the future. And I agree with the point about other tables - the old design was leaving orphaned data in tables other than events_v2 whereas the PendingStatsDeletion list is a single signal to sweep all tables which is definitely a nice property.

Expanding the old design for all tables comes with its own set of issues and I'm happy to move forward with the PendingStatsDeletion approach proposed here as long as the questions about stats boundaries and partition calculation are resolved.

I failed to see the "reset stats" scenario and the fact that we should not estimate partition range based on imported data.

I'll add more on these.

With regards to partitioning lightweight deletes, my understanding was it's always helpful to chunk work, especially given that ClickHouse was running mutations (on 202507) for a partitioned lightweight delete in a test @apata (https://github.com/apata) conducted:

Let's also discuss this inline

@aerosol
aerosol force-pushed the delete-site-improve branch from c4e6048 to 431e6c8 Compare August 13, 2026 06:10
@aerosol

aerosol commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@ukutaht @cnkk @apata pushed 431e6c8 and got rid of wobbly date markers dependency. This is still under assumption we should chunk lightweight deletes by partitions, and it is now using the method suggested by Cenk. I'm not sure about lightweight_deletes_sync = 0, it seems as if lightweight delete Artur tried before ran synchronously with the current setting. I'm puzzled.

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.

4 participants