fix: drop stale `__rill_temp_*` ClickHouse tables left behind by interrupted model runs by nishantmonu51 · Pull Request #10025 · rilldata/rill

nishantmonu51 · 2026-10-15T19:32:33Z

While writing a model, the ClickHouse driver creates __rill_temp_<table>_<hash> tables: one per partition for partition_overwrite inserts and a view for column inference. They are only dropped in a defer with a 15 s budget, so they stay behind when the runtime restarts mid-run or when the drop fails because a cancelled insert still holds the table lock. Nothing swept them afterwards.

Open now starts a background collector for readwrite connections. It sweeps once immediately and then at most hourly, and drops every __rill_temp_* table in the project database whose metadata_modification_time is older than 24 hours. The default model timeout is 3 hours, so running executions are not affected. A database that can scale to zero is only swept while it is in use, so the collector never wakes it up.
Temp table creation and the sweep are serialized with tempTableMu, since temp table names are deterministic: a table created before the sweep listed the stale ones is not stale, and one created after it waits for the drops to finish.
dropTable in cluster mode now also drops a _local table whose Distributed table was never created, which previously stranded the _local half of a temp table when the execution was cancelled between the two CREATEs.

Steps to reproduce:

Create a ClickHouse project with an incremental model that uses incremental_strategy: partition_overwrite and several partitions.
Stop the runtime while the model is refreshing, or simulate the leftover with CREATE TABLE __rill_temp_<model>_<32 hex chars> AS <model>.
Run SELECT name FROM system.tables WHERE name LIKE '__rill_temp_%'. Before this change the leftover table stays until the same partition happens to run again; now the collector drops it within an hour of it turning 24 hours old and logs dropped stale temp table.

Closes APP-947

Checklist:

Covered by tests
Ran it and it works as intended
Reviewed the diff before requesting a review
Checked for unhandled edge cases
Linked the issues it closes
Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
Intend to cherry-pick into the release branch
I'm proud of this work!

…rrupted model runs The ClickHouse driver creates `__rill_temp_<table>_<hash>` tables while writing a model (one per partition for partition_overwrite inserts, a view for column inference) and only drops them in a defer with a 15 s budget. The drop never runs if the runtime restarts mid-run and fails when a cancelled insert still holds the table lock, so the tables accumulated in the project database. Add dropStaleTempTables, which drops the temp tables of a model's table (including its staging name and `_local` counterparts in cluster mode) at the start of every execution, on Rename and on Delete. Temp tables that executions in this process are writing to are tracked in Connection.tempTables and skipped. In cluster mode, dropTable now also drops a `_local` table whose Distributed table was never created.

k-anshul

The solution feels somewhat complex for a garbage collection. Sweeping on every execute can be wasteful for models with large number of partitions.
As I mentioned offline the temp table names are stable so this is usually never required.

However if we still want to handle it, checking once in Open should be enough. It is guaranteed that max one connection is opened at a single time so there is no risk of concurrent execution.
If we want faster garbage collection and faster Open than I suggest a background garbage collector instead. Open would start a goroutine, only in readwrite mode, that runs once immediately and then hourly, and drops every _rill_temp* table older than 24 hours:

SELECT name FROM system.tables
WHERE database = currentDatabase()
  AND startsWith(name, '__rill_temp_')
  AND metadata_modification_time < now() - INTERVAL 24 HOUR

The default model execution timeout is 3h so 24h gives us a good margin as well.

Sweeping on every execution added a metadata query per partition run and needed an in-flight registry to protect concurrent partition inserts. Instead, Open now starts a goroutine for readwrite connections that runs once immediately and then hourly, and drops every __rill_temp_* table whose metadata_modification_time is older than 24 hours. The default model timeout is 3 hours, so running executions are not affected. The hooks in Execute, Rename and Delete and the registry are removed; the `_local` fallback in dropTable stays since it also fixes the in-flight cleanup of cancelled cluster inserts.

k-anshul

There is a rare but non-zero chance that list returns a table name and then a reconciler recreates it and then drop drops it while a concurrent reconcile is running.

k-anshul · 2026-10-16T11:38:51Z

+			c.logger.Warn("clickhouse: failed to drop stale temp table", zap.String("name", name), zap.Error(err), observability.ZapCtx(ctx))
+			continue
+		}
+		c.logger.Info("clickhouse: dropped stale temp table", zap.String("name", name), observability.ZapCtx(ctx))


better to skip the log on ErrNotFound from correctness pov.

k-anshul · 2026-10-16T11:39:56Z

+	}
+	for _, name := range names {
+		err := c.dropTable(ctx, name)
+		if err != nil && !errors.Is(err, drivers.ErrNotFound) {


Nit: Can return early on ctx canceled.

k-anshul · 2026-10-16T11:43:55Z

+
+	for {
+		// Skip if the database hasn't been used recently and may be scaled to zero, so that the sweep does not wake it up.
+		if !c.config.CanScaleToZero || time.Since(c.lastUsedOn()) < 2*time.Minute {


Looks like this was copied from periodicallyEmitStats but that runs at a faster schedule whereas this runs at an hourly schedule, there is a good chance most schedules may miss it.

But waking up cluster to ensure garbage collection may be wasteful so may need to rethink. It also advances the used tick.

I agree waking up the cluster seems wasteful and its ok if it doesn't end up running for few hrs or even days.. what we want to prevent is these tables silently piling up.

Changed to a minute ticker that sweeps once an hour has passed and the DB was used in the last 2 min, so a due sweep runs as soon as the DB is awake, without waking it.

…r loop Temp table names are deterministic, so an execution could recreate a stale table after the sweep listed it and before it dropped it. Creation now holds tempTableMu for reading and the sweep holds it for writing across the listing and the drops. The sweep skips the log on ErrNotFound, returns early when the context is cancelled, and the collector ticks every minute but sweeps at most hourly, so a due sweep runs as soon as a database that can scale to zero is in use again instead of waiting for an hourly tick to coincide.

nishantmonu51 · 2026-10-16T12:08:17Z

Switched to the collector as suggested; kept the _local fallback in dropTable since it also fixes the in-flight cleanup of cancelled cluster inserts.

Race fixed with tempTableMu: temp table creation takes the read lock and the sweep holds the write lock across the listing and the drops, so a table recreated after the listing waits for the drops.

k-anshul · 2026-10-16T12:27:28Z


+	// tempTableMu serializes the creation of temporary tables (read lock) with the sweep of stale ones (write lock).
+	// Temporary tables have deterministic names, so without it an execution could recreate a stale table after the sweep listed it and before it dropped it.
+	tempTableMu sync.RWMutex


Use runtime/pkg/ctxsync/rwmutex.go

nishantmonu51 requested a review from k-anshul October 15, 2026 19:32

k-anshul requested changes Oct 16, 2026

View reviewed changes

nishantmonu51 requested a review from k-anshul October 16, 2026 12:08

k-anshul approved these changes Oct 16, 2026

View reviewed changes

Use the context-aware ctxsync.RWMutex for tempTableMu

c2effd8

nishantmonu51 merged commit 5b11c5b into main Oct 19, 2026
12 checks passed

nishantmonu51 deleted the nishant/clickhouse-temp-table-cleanup branch October 19, 2026 05:54

k-anshul mentioned this pull request Oct 19, 2026

fix: unique ClickHouse temp table names instead of tempTableMu #10036

Merged

8 tasks

Navigation Menu

Repository navigation

fix: drop stale `__rill_temp_*` ClickHouse tables left behind by interrupted model runs - #10025

fix: drop stale `__rill_temp_*` ClickHouse tables left behind by interrupted model runs#10025
nishantmonu51 merged 4 commits into
mainfrom
nishant/clickhouse-temp-table-cleanup

nishantmonu51 commented Oct 15, 2026 •

edited

Loading

Uh oh!

k-anshul left a comment •

edited

Loading

Uh oh!

k-anshul left a comment

Uh oh!

k-anshul Oct 16, 2026

Uh oh!

nishantmonu51 Oct 16, 2026

Uh oh!

k-anshul Oct 16, 2026

Uh oh!

nishantmonu51 Oct 16, 2026

Uh oh!

k-anshul Oct 16, 2026 •

edited

Loading

Uh oh!

nishantmonu51 Oct 16, 2026

Uh oh!

nishantmonu51 Oct 16, 2026

Uh oh!

nishantmonu51 commented Oct 16, 2026

Uh oh!

k-anshul Oct 16, 2026

Uh oh!

nishantmonu51 Oct 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nishantmonu51 commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k-anshul left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-anshul left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-anshul Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nishantmonu51 commented Oct 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nishantmonu51 commented Oct 15, 2026 •

edited

Loading

k-anshul left a comment •

edited

Loading

k-anshul Oct 16, 2026 •

edited

Loading