fix(branch): delete safekeeper WAL on branch delete, not just pageserver - #2
Merged
zxzinn merged 1 commit intoAug 17, 2026
Merged
Conversation
branch delete only called the pageserver's DELETE /timeline endpoint. Each safekeeper in the quorum independently persists a timeline's WAL and is never notified, so its on-disk directory becomes a permanent orphan on every delete. This filled all three safekeepers' 5Gi volumes to 100% on the maiagent-dev-neon cluster after roughly 60 failed refresh-golden runs each left one behind, which then made every new branch creation fail with "No space left on device". Adds an internal/safekeeper HTTP client and calls DELETE on each safekeeper listed in the new --safekeeper-http-urls flag after the pageserver delete succeeds. Kept as a separate flag from --safekeepers (which carries in-cluster host:port for compute's neon.safekeepers config) since the CLI process itself usually cannot resolve *.svc.cluster.local names — reaching a safekeeper's HTTP API requires the caller to already have a path to it (e.g. a port-forward) and pass the resulting URL. The flag is optional and best-effort: a safekeeper cleanup failure is logged as a warning but does not fail the command, since the pageserver deletion already happened and is not reversible.
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
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.
Summary
branch deleteonly called the pageserver'sDELETE /timelineendpoint. Each safekeeper in the quorum independently persists a timeline's WAL and is never notified, so its on-disk directory becomes a permanent orphan on every delete.Found this on the maiagent-dev-neon cluster: roughly 60 failed
refresh-goldenCI runs each left one orphaned timeline behind, filling all three safekeepers' 5Gi volumes to 100% and making every subsequent branch creation fail withNo space left on device.Changes
internal/safekeeperpackage: a thin HTTP client mirroringinternal/pageserver's style, calling the safekeeper'sDELETE /v1/tenant/{tenant}/timeline/{timeline}.branch deletenow calls this on every safekeeper in the new--safekeeper-http-urlsflag, after the pageserver delete succeeds.--safekeepers(which carries in-clusterhost:5454for compute'sneon.safekeepersconfig, resolved pod-to-pod). The CLI process itself usually cannot resolve*.svc.cluster.localnames, so reaching a safekeeper's HTTP API requires the caller to already have a path to it (e.g. onekubectl port-forwardper safekeeper) and pass the resultinghttp://localhost:PORTURLs.Test plan
go build ./...,go vet ./...,go test ./...all pass--safekeeper-http-urlspointing at threekubectl port-forwarded safekeepers, confirmed the timeline directory was actually removed from all three (kubectl exec ... ls /data/<tenant>/) — before the fix it was left behind on all three.branch deletewithout--safekeeper-http-urlsstill deletes the branch normally (matches pre-fix behavior for anyone not yet passing the new flag).