Skip to content

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

Open
bjmeetsfo wants to merge 2 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 2 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.

set_namespace_state checked "namespace still holds a live table" under a read
lock, released it, then recorded and applied the change. A table created in
that window was stranded: the namespace reached Dropped with a live table
still inside it, which is what the check exists to prevent. The metaserver
serves each connection on its own thread, so a drop and a table creation
arriving together is ordinary traffic. It is the same end state as the
raft-path defect, reached differently -- there the guard was absent, here it
was released too early.

The check, the record and the apply now happen under one write lock. The
apply body moves into apply_namespace_state_locked, taking an already-held
&mut MetaState, so the guarded path can check and apply without letting go
while replay keeps entering through apply_set_namespace_state and reapplying
unconditionally.

record_mutation stays before the state moves, so a crash between them
replays the change rather than losing it, and it does not touch self.inner
so holding the lock cannot deadlock.

Holding the lock across record_mutation means one fsync with readers
blocked. That is affordable here and nowhere else: the only callers are
freeze, unfreeze and drop of a namespace, which are operator actions rather
than a background loop or a per-request path. This was also the only method
in the metaserver that released its lock between checking and applying.
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.

Dropping a namespace with a group routing to it succeeded and left the
contradiction standing: the group stayed Normal, its proxies stayed
attached, and the very next proxy heartbeat still handed out the dropped
namespace to serve. Calibration does not clean this up, because it keys
off the group's own state and nothing changed it. No report flags it
either, so the tier goes on routing to a namespace that is gone.

Count a live proxy group as a live dependent, refused with
namespace_still_routed. This keeps the existing order of operations --
drop the group, then drop the namespace -- and 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; dropping the
group first still lets the namespace go, so the guard lets go; and
freeze/unfreeze, which share this path, are untouched.
@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
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.

2 participants