Repository navigation
Conversation
ggevay
force-pushed
the
gabor/sql-766-cloning-a-compute-read-hold-after-drop-cluster-aborts
branch
2 times, most recently
from
October 9, 2026 10:24
bf41b48 to
f74e094
Compare
A compute read hold reports changes to its compute instance, and `ReadHold::clone` panics once that channel is closed. `DROP CLUSTER` shuts the instance down and closes it, while holds on the cluster can still be in use, so cloning one aborted environmentd: - A fast-path peek acquires its read holds before optimization and cloned the hold on the index it peeks after it, so a `DROP CLUSTER ... CASCADE` during the optimization panicked the session task. - `Command::GetTransactionReadHoldsBundle` cloned the transaction's stored holds on the coordinator. `DROP CLUSTER` does not remove them, so a SELECT in a transaction whose cluster was dropped after the SELECT took its catalog snapshot panicked the coordinator. The fast path now takes the hold instead of cloning it, and the peek then fails with the existing "cluster ... was dropped" error. The command clones the holds with the new `ReadHolds::try_clone`, which maps a hung-up issuer to `ConcurrentDependencyDrop`, and the frontend returns that error. `ReadHolds::subset`, which the frontend applies to the fetched holds, now moves them instead of cloning, since the instance can shut down right after the fetch. The `ReadHold` docs now name a dropped compute instance as a cause of a hung-up issuer, besides process shutdown. Fixes SQL-766. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`ReadHold::clone` panicked when the hold's issuer had hung up, and the issuer of a compute hold is its compute instance, which `DROP CLUSTER` shuts down in normal operation. The panic hid behind a standard trait: `#[derive(Clone)]` on `ReadHolds`, `Option::cloned` and collection clones all cloned holds without saying so, which is how the previous commit's crashes came about. `ReadHold` and `ReadHolds` no longer implement `Clone`, and `merge_assign` returns an error instead of panicking, so every site has to say what a hung-up issuer means there: - The frontend's copy of its holds for `StoreTransactionReadHolds` uses `try_clone`, and the command, `ReadHolds::merge` and `store_transaction_read_holds` return the error, so the statement fails with "cluster ... was dropped". - The bounded-staleness metric skips its comparison. - The compute instance's dataflow creation clones the holds on its inputs with `clone_input_read_hold`, which still panics: the inputs are storage collections and indexes of the same instance, whose issuers only hang up during process shutdown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two tests with new failpoints, each parking a SELECT on cluster `c`, running `DROP CLUSTER c CASCADE`, and resuming it: - `test_fast_path_peek_after_concurrent_cluster_drop` parks a fast-path SELECT at `peek_before_optimize`, after it acquired its read holds. - `test_transaction_read_holds_after_concurrent_cluster_drop` parks the second SELECT of a transaction at `txn_read_holds_before_dispatch`, before it fetches the transaction's read holds. The SELECTs fail with a "was dropped" error and environmentd stays up. Without the previous commit, the first test's session task panics, which closes the connection, and the second test's coordinator panics. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ggevay
force-pushed
the
gabor/sql-766-cloning-a-compute-read-hold-after-drop-cluster-aborts
branch
from
October 9, 2026 11:12
f74e094 to
b5b3d22
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixes SQL-766.
DROP CLUSTERshuts the cluster's compute instance down, and cloning a compute read hold on it afterwards panics (cannot clone ReadHold: read hold issuer ... has hung up), aborting environmentd. Two places did that during a concurrentDROP CLUSTER: a fast-path SELECT cloning the hold on the index it peeks, andGetTransactionReadHoldsBundlecloning a transaction's stored holds.Description
ReadHolds::try_clone, andReadHolds::subsetmoves them on, since the instance can shut down right after the copy. Both statements fail with "cluster '...' was dropped".ClonefromReadHoldandReadHolds. Derived and implicit clones hid the panic, so every remaining copy now states what a hung-up issuer means there:merge_assignand its callers) return the error.clone_input_read_hold, since its inputs' issuers only hang up during process shutdown.DROP CLUSTER, and resume it: a fast-path peek, and the second SELECT of a transaction. Both fail with "was dropped", and environmentd stays up. Without commit 1, both failed in 10 of 10 local runs.🤖 Generated with Claude Code