Skip to content

A namespace the routing tier still serves is not empty - #299

Open
bjmeetsfo wants to merge 5 commits into
mainfrom
oss/a-routed-namespace-is-not-empty
Open

A namespace the routing tier still serves is not empty#299
bjmeetsfo wants to merge 5 commits into
mainfrom
oss/a-routed-namespace-is-not-empty

Conversation

@bjmeetsfo

@bjmeetsfo bjmeetsfo commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Dropping a namespace already refuses while a table in it is still live, so
that dropping one cannot strand a table. A proxy group is the other thing
that can still depend on a namespace, and nothing counted it.

What happens today

Declare a namespace, put a proxy group on it, calibrate so a proxy attaches,
then drop the namespace. Measured on main:

attach-before-drop = [("p1", "orders")]
drop_namespace     -> ok=true code=ok
attach-after-drop  = []      release-after-drop = []
group orders  ns=ns  state=Normal  instance_num=1
heartbeat -> namespace="ns"  config_changed=true
degraded_reasons = []

The drop succeeds and the contradiction stays standing. The group is still
Normal, its proxy is still attached, and the very next heartbeat still hands
that proxy the dropped namespace to serve. Calibration does not clean it up,
because it keys off the group's own state and nothing changed that. No report
flags it either, so the routing tier goes on routing to a namespace that is
gone.

The change

Count a live proxy group as a live dependent of the namespace, refused with
namespace_still_routed. That keeps the order of operations the code already
implies -- drop the group, then drop the namespace -- and drop_proxy_group
already releases the proxies through the ordinary calibration path.

The check sits in the same write-locked block as the table check, so it cannot
race a group created alongside the drop.

Tests

  • a routed namespace is refused, and nothing moves: the group stays Normal
    and the proxy keeps its assignment.
  • dropping the group first still lets the namespace go, so the guard lets go
    rather than making a routed namespace permanently undroppable.
  • freeze and unfreeze share this path and are untouched.

The first test fails on the unmodified code with
a namespace with a proxy group routing to it was dropped.

Base and a merge hazard worth knowing about

This is stacked on #285, which restructured the same function to hold one
write lock across the check and the apply. Merge #285 first.

It targets main rather than #285's branch on purpose: rust-ci and
oss-readiness both declare pull_request: branches: [main, rust-main], and
for pull_request that filters the base. A pull request aimed at any other
branch therefore gets only auto-approve, SPDX and gitleaks -- it is never
compiled and its tests are never run, while still reporting CLEAN. Targeting
main means the diff here also carries #285's commit, and CI builds the two
together, which is the state that will actually land. Once #285 merges, this
diff reduces to its own commit on its own.

Separately, and worth flagging because it affects #285 and #231 whether or not
this change exists: #285 and #231 conflict with each other, even though
GitHub reports both as mergeable against main -- they are only ever compared
against main, never against each other. Merging #285 and then #231 gives:

CONFLICT (content): Merge conflict in crates/temporalstore-rust/src/meta/registration.rs

The resolution needs care. #231 moves the emptiness check into
admission_refusal, which takes self.inner.read(). #285 holds
self.inner.write() across that same region. Resolving by simply taking
#231's side puts a read acquisition inside the write lock, on the same thread
and the same lock. Only the Dropped arm of admission_refusal takes that
lock, so freeze and unfreeze would look fine and dropping a namespace would
hang.

That is measured, not predicted. Resolving it that way and running the
namespace drop test gives:

running 1 test
test meta::tests::the_emptiness_check_still_refuses_and_still_lets_go ...
EXIT_CODE=124        <- killed by a 45s timeout; the test never returns

The resolution that works is to factor the guard so it takes &MetaState:
the public path calls it with the write guard it already holds, and the
propose path acquires its own read lock. If that factoring lands in #231,
the guard added here should move into it too, so the raft propose path
refuses a routed namespace as well -- today it bypasses this check exactly
as it bypasses the table check.

@bjmeetsfo
bjmeetsfo requested a review from superhaiou as a code owner August 25, 2026 07:08
@bjmeetsfo
bjmeetsfo changed the base branch from oss/close-the-namespace-drop-window to main August 25, 2026 07:38
@bjmeetsfo bjmeetsfo closed this Aug 25, 2026
@bjmeetsfo bjmeetsfo reopened this Aug 25, 2026
supermengm and others added 4 commits August 25, 2026 02:23
The namespace state change has to decide and act without letting go: the
emptiness check used to run under a read lock that was released before the
change was applied, and a table created in that window was stranded with
its namespace already Dropped.

The judgement it needs now lives in admission_refusal, which takes its own
read lock. Calling that under the write lock this path must hold is a read
acquired inside a write, on the same lock and the same thread, and it does
not return -- measured, the namespace drop test never finishes.

So each judgement gains a form that takes the state to read rather than
fetching it: admission_refusal_in, reserved_name_refusal_in and
namespace_not_empty_in. The propose path takes a read lock and calls it;
the namespace path passes the write guard it already holds. add_namespace
and add_table now go through admission_refusal too, so one judgement
serves every caller.
…tart' into tmp/compose-235

# Conflicts:
#	crates/temporalstore-rust/src/meta/registration.rs
Dropping a namespace refuses while a table in it is still live, so that
dropping one cannot strand it. A proxy group is the other thing that can
still depend on a namespace, and nothing counted it: the drop succeeded,
the group stayed Normal, its proxies stayed attached, and the very next
heartbeat still handed them the dropped namespace to serve.

The check sits in admission_refusal beside the table check it belongs
with, so the raft propose path refuses a routed namespace as well -- the
earlier inline version guarded only the public method.
@bjmeetsfo
bjmeetsfo force-pushed the oss/a-routed-namespace-is-not-empty branch from 648ea40 to 5915cb7 Compare August 26, 2026 08:06
# Conflicts:
#	crates/temporalstore-rust/src/meta.rs
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.

3 participants