Reduce site deletions impact on ClickHouse - #6591
Conversation
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- Reset stats today (
Plausible.Purge.reset!/1bumpsSite.native_stats_start_dateto2026-08-12) - Delete the site
- Expected result:
PendingStatsDeletionis inserted withstats_start_date: 2026-01-01 - Actual result: depends on whether any events were ingested after stats were reset
4a. If no events received: noPendingStatsDeletioninserted at all becausePlausible.Sites.stats_range/1returns{nil, nil}
4b. If some events received:PendingStatsDeletionis inserted withstats_start_datebeing the date of the first event received after2026-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), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think partition_ids could be simplified by querying Clickhouse for the partitions
SELECT DISTINCT _partition_id FROM events_v2 WHERE site_id IN (…)There was a problem hiding this comment.
@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.
| PlausibleMetrics.measure_duration(telemetry_stage_duration(), fun, %{stage: stage}) | ||
| end | ||
|
|
||
| defp clear_partitioned_table!(table, partition_id, site_ids) do |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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.
|
Some good points, especially the stats dates quirk that you both noticed. 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:
I also started thinking of ClickHouse queue as of configurable self-managing capacity rather than limitation. Anyway:
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. Moreover I thought of reusing that table for handling expired trials/abandoned accounts ahead of time (different
I like it, we can try that, but:
It sounds nice on paper but the previous design wasn't querying 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?
What do you mean by "risk of breaking ingestion" here? The fact that current clean up worker uses |
| 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 |
There was a problem hiding this comment.
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), |
There was a problem hiding this comment.
I think partition_ids could be simplified by querying Clickhouse for the partitions
SELECT DISTINCT _partition_id FROM events_v2 WHERE site_id IN (…)|
@aerosol Thanks. Getting to the heart of the discussion on the
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 Expanding the old design for all tables comes with its own set of issues and I'm happy to move forward with the
I'll add more on these.
Let's also discuss this inline |
c4e6048 to
431e6c8
Compare
|
@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 |

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:
timestampvsstart). Classifying this as negligible for now.Tests
Changelog
Documentation
Dark mode