Skip to content

Never report a namespace whose entry was emptied as missing - #56

Open
tszymczyszyn-shopify wants to merge 17 commits into
v0.9.30-shopify-patchesfrom
tszymczyszyn/no-false-namespace-not-found
Open

tszymczyszyn-shopify wants to merge 17 commits into
v0.9.30-shopify-patchesfrom
tszymczyszyn/no-false-namespace-not-found

Conversation

@tszymczyszyn-shopify

@tszymczyszyn-shopify tszymczyszyn-shopify commented Oct 14, 2026 •

Copy link
Copy Markdown

Problem

When libsql-server shuts down, or drops a namespace from its in-memory cache, a request that is already waiting for that namespace can be told that the namespace doesn't exist: HTTP 404, or gRPC FAILED_PRECONDITION "namespace doesn't exist". The namespace is fine. It is still on disk and in the metastore. The server only stopped holding it in memory while the request was waiting.

Why it happens. Each loaded namespace sits in a cache entry. Three things take the namespace out of its entry: shutdown, cache eviction and destroy. A request that picked up the entry just before that finds it empty, and NamespaceStore::with reports an empty entry as "doesn't exist". Only after destroy is that true.

When it happens. On every rolling restart, because the server tears the namespace store down while it is still accepting requests. Under cache pressure it can also happen at any time. In our production deployment it showed up as several hundred Namespace `<name>` doesn't exist errors in one week of rolling restarts and node drains, all for namespaces that existed. Against a local server, the harness in #58 reproduces it at will: with the cache capacity below the number of hot namespaces, 5.9% of answers were false 404s; a SIGTERM under load produced 13 to 41 of them each time.

Why it matters. A 404 tells the client the namespace is gone, and clients act on that, for example by creating it again or discarding local state. A temporary error would have been retried. Embedded replicas built on the libsql crate are an example: their replicator retries connection and UNAVAILABLE errors on its own, but passes "namespace doesn't exist" straight up to the application as a sync() failure.

Two smaller problems in the same code:

  • After shutdown had finished, an admin request could load the namespace from disk again.
  • "Doesn't exist" also decides how gRPC requests are authenticated. For a missing namespace, the replication service falls back to the server-wide auth settings, and the proxy service falls back to the auth context sent with the request. A false "doesn't exist" sent requests for existing namespaces down those paths.

Upstream tracks the shutdown case as tursodatabase#1772.

Change

  • with
    • Returns NamespaceStoreShutdown (503) once the store has shut down, on every path.
    • When the entry it got is empty, it looks the namespace up again, up to 3 times, so an evicted namespace is reloaded. An entry that stays cached but empty (a fork that failed before storing its record leaves one behind) is found on every attempt; see "Not in scope".
    • When it gives up, it answers NamespaceDoesntExist only if the metastore no longer has the namespace. Otherwise it answers NamespaceStoreShutdown, or the new NamespaceUnavailable (503).
  • load_namespace checks has_shutdown again inside its init, which runs on a cache miss only, so one check covers with, create and fork. The error comes back wrapped as Error::Ref. reset checks the flag at its top. A load that was already opening the namespace when shutdown started can still finish; closing that is the shutdown-order follow-up.
  • fork loads its source through with and uses the same error rule.
  • create
    • Refuses after shutdown, like destroy and fork.
    • If shutdown starts between storing the record and loading the namespace, the load is refused before anything is opened or written, and create removes the record again. Left in place, every retry answered "already exists" for a namespace that was never loaded, so a dump was never applied.
  • checkpoint no longer inserts an empty cache entry for a namespace that isn't loaded. Before, a checkpoint of an unloaded namespace made that namespace answer 404 until the entry was evicted.
  • gRPC
    • rpc::namespace_error_status maps NamespaceDoesntExist to the existing FAILED_PRECONDITION, and NamespaceStoreShutdown and NamespaceUnavailable to UNAVAILABLE. Anything else stays INTERNAL. FAILED_PRECONDITION is only ever a missing namespace.
    • replication::auth::authenticate, ProxyService::extract_context and ReplicaProxyService::do_auth decide on the code it returns: only a missing namespace falls back to the user strategy (or skips authentication, in the proxy). So the error is classified, and Error::Ref unwrapped, in one place, and that place is tested with wrapped errors. Before, a Hello during shutdown got INTERNAL "Error fetching jwt key for a namespace: Namespace store has shutdown", and a replica shutting down answered INTERNAL from its proxy.
    • Error::as_ref unwraps nested Refs, as the HTTP mapping already did.
  • Observability
    • libsql_server_namespace_emptied_entry: each time a request finds its entry emptied and looks again. Rises on every deploy; informational.
    • libsql_server_namespace_emptied_entry_gave_up: requests answered NamespaceUnavailable because every lookup found an emptied entry while the namespace still exists. Giving up because the store shut down, or because the namespace is really gone, is not counted. This is the only new user-visible failure; alert on it.
    • The retry is logged at debug, giving up at warn.

What embedded replicas see

Measured with eight embedded replicas (the libsql crate) syncing in a tight loop against a primary that is stopped and started again (replica-sigterm.sh in #58):

Base This branch
At the signal sync() fails with Requested namespace doesn't exist (Error::NamespaceDoesntExist). The replicator does not retry it; the application gets the error. The replicator logs error connecting to primary. retrying. error: status: Unavailable, message: "Namespace store has shutdown" and retries the handshake itself, with 1 s sleeps, up to its retry budget.
While the server is down Timeout performing handshake with primary once the retries run out, same as before. Same.
After the restart Syncs resume. Syncs resume.

So for a client built on the crate, a primary restart that is shorter than the replicator's handshake retries is now absorbed inside sync(). Before, every restart surfaced as a "namespace doesn't exist" failure to the application, which is the one error it might reasonably act on by discarding its local replica.

How to review

About 240 lines are production code, comments included; the rest is tests.

Reading order:

  1. NamespaceStore::with and emptied_entry_error in namespace/store.rs. This is the fix.
  2. The shutdown check inside load_namespace's init, the ones at the top of create and reset, and load_created.
  3. The one-line changes in fork and checkpoint.
  4. rpc::namespace_error_status and its call sites in proxy.rs, replica_proxy.rs, replication_log.rs and replication/auth.rs.
  5. The tests. Skim them, apart from the comments on start_waiting and settle.

Invariants worth checking:

  • Shutdown first. has_shutdown is set before shutdown() empties any entry. with checks it at the start of every attempt, and load_namespace checks it before opening a namespace. A request that wakes on an entry emptied by shutdown gets a 503. The ordering between the flag and the entries comes from the entry's RwLock: the flag store is sequenced before shutdown's write-unlock, and the request's read-lock acquires after it, so the Relaxed loads and stores are enough. The one remaining gap is a load that was already inside make_namespace when the flag was set.
  • 404 only from the metastore. with returns NamespaceDoesntExist only when the metastore has no record. The remaining exception is the MetaStore::handle resurrection listed under follow-ups.
  • Nothing fills an entry it found. A request that finds an emptied entry looks the namespace up again and gets whatever the cache holds then.
  • No lock held across cache calls, in with. fork and reset hold an entry write lock across a cache call; that predates this change and is commented in the code, with the fix left to the follow-ups.
  • Only a missing namespace changes how a request is authenticated. The three gRPC auth sites fall back (or skip authentication) on FAILED_PRECONDITION from namespace_error_status and on nothing else.
  • Test synchronization. moka keeps an extra reference to a fresh entry until its housekeeping runs. The tests therefore settle the cache before they rely on reference counts.

Tests

namespace::store::test, rpc::test, rpc::proxy::test, rpc::replica_proxy::test and rpc::replication::replication_log::test add 21 tests. They drive each case deterministically: the test holds the entry's write lock, so the request waits on the same entry, then empties it the way each caller does. Covered:

  • with after shutdown, and shutdown starting while a request waits: NamespaceStoreShutdown. Before, the first reloaded the namespace.
  • An entry evicted while a request waits is reloaded into a fresh entry; the emptied entry is not filled. Before: NamespaceDoesntExist.
  • Eight requests waiting on one evicted entry all succeed and share one reloaded namespace: the lookups coalesce into one load (multi-thread runtime).
  • An entry that stays cached but empty: NamespaceUnavailable after the three attempts, through the real with, and the entry is still the cached one.
  • A namespace destroyed while a request waits, against the real destroy: still NamespaceDoesntExist.
  • load_namespace refuses to open a namespace once the store has shut down; reset and create after shutdown are refused.
  • create whose load shutdown refused removes its record; a retry against a fresh store on the same files creates the namespace.
  • fork reloads a source evicted while it waited; a source emptied between its two loads answers NamespaceUnavailable.
  • checkpoint of an unloaded namespace caches nothing.
  • The emptied_entry_error outcomes.
  • Hello (external and internal service), ProxyService::extract_context and ReplicaProxyService::do_auth after shutdown: UNAVAILABLE. The Hello test configures Basic auth and sends no credentials, and the proxy test sends an x-proxy-authorization header, so each fails if the fallback for the two 503 errors ever returns.
  • The status mapping, including wrapped and doubly wrapped errors and the NAMESPACE_DOESNT_EXIST message.
  • HTTP 503 and 404 for the respective errors, wrapped or not.
  • The two 503 messages never contain "doesn't exist" or "not found": clients that match on the message must not read them as a 404.

Mutation checks: putting the original NamespaceDoesntExist back on the give-up path, removing either shutdown check, dropping the record removal in create, letting the UNAVAILABLE case fall back to the user strategy, unwrapping only one Ref, swapping the two steps inside destroy, and restoring the proxy fallback each fail the test added for it. The concurrency tests passed 300 consecutive runs before the last restructuring and 30 after it.

cargo nextest run -p libsql-server: 225 tests, 224 passed, with the same results as the base branch.

  • On macOS, embedded_replica::local::local_sync_with_writes and embedded_replica::replica_no_resync_on_restart fail on both the base and this branch under parallel load. replica_no_resync_on_restart passes when run alone.
  • cluster::replication::apply_partial_snapshot and connection_core::test::test_many_concurrent are flaky under parallel load on both.

Verified against running servers

Debug sqld built from this branch and from the base, run locally with --enable-namespaces and driven over Hrana HTTP by the scripts in #58.

Scenario Base This branch
Eviction churn: --max-active-namespaces 2, 6 hot namespaces, 24 clients, 20 s 35,074 × 200, 2,197 × 404 (5.9%) for namespaces that exist 29,807 × 200, 0 non-200. 1,504 emptied entries retried, all succeeded; no give-ups.
SIGTERM under load: 20 namespaces, 32 clients, signal at 3 s, 3 runs after the signal: 13 to 41 × 404, then connection errors; first 404 at +3 ms after the signal: 0 × 404, 46 to 62 × 503 Namespace store has shutdown, then connection errors; first 503 at +1 ms

| Embedded replica (the libsql crate): 8 replicas on 8 namespaces syncing in a tight loop, 20 namespaces loaded, SIGTERM at 4 s, restart at 7 s | sync() failed at the signal with Requested namespace doesn't exist (Error::NamespaceDoesntExist, not retried) | never "doesn't exist". sync() either surfaced status: Unavailable, message: "Namespace store has shutdown" or, because the client retries a Client error's handshake with 1 s sleeps, ended in Timeout performing handshake with primary once the server had been down for the retry budget; all replicas resumed after the restart |
| 503 window versus loaded namespaces: SIGTERM under load with every namespace holding a WAL | | 2,000 namespaces: first 503 at +4 ms, port closed at +0.41 s (server log 0.48 s). 5,000 namespaces: +1.52 s (server log 1.70 s). About 0.3 ms per loaded namespace in a debug build; the work per namespace is task shutdown and sentinel removal, not data size, since --snapshot-at-shutdown is off |

The 65-check regression matrix (regress.sh in #58) ran against both binaries. Only the intended lines differ:

  • Checkpoint of an unloaded namespace, then query: base 404 (the namespace was unreachable until eviction), this branch 200.
  • SIGTERM under load with namespaces disabled: base 1 × 404, this branch 0 (503 only).
  • Replica SIGTERM under read load: base 12 × 404, this branch 0 (503 only).
  • Delete under load (S6) is a race on both binaries. With requests in flight, destroy sometimes leaves data-wal behind (DirectoryNotEmpty, two 500 disk I/O error answers) and, on the base, requests sometimes still got 200 after the delete. In a reviewer's 10 runs per binary the directory stayed in 6 of 10 on the base and 2 of 10 here, where requests always got 404 after the delete; in the run above the base left it and this branch did not. The race is the MetaStore::handle resurrection under follow-ups; this change does not add to it and shows it less often.

Identical on both: namespace lifecycle (create, duplicate 400, query, config, stats, delete, real 404s, directory removed); fork with hot and cold source, unknown source 404, loaded destination 400; create from a file:// dump; shared-schema create with hot and cold schema source, linked-db delete guard; data intact after SIGTERM restarts with and without --snapshot-at-shutdown; replication catch-up, proxied writes through the replica, replication and proxied writes resuming after a primary restart; a replica request for an unknown namespace answering 404 through the NAMESPACE_DOESNT_EXIST string match; mixed versions in both directions catching up and recovering after a primary restart. The replica log showed the new mapping live: error connecting to primary. retrying. error: status: Unavailable, message: "Namespace store has shutdown", followed by recovery.

Not exercised live: the proxy-side UNAVAILABLE (the shutdown window is about 10 ms); it is covered by the unit test.

Rollout

  • The rollout itself still runs the old binary on every pod it stops. Those pods answer the false 404 one last time while they shut down. Clients that act on a 404 should hold off on that during this rollout.
  • What clients see during restarts afterwards:
    • HTTP: 503 instead of 404.
    • gRPC: UNAVAILABLE instead of FAILED_PRECONDITION or INTERNAL.
    • Embedded replicas (the libsql crate): sync() fails with a transport error carrying UNAVAILABLE instead of Error::NamespaceDoesntExist. An application that matched on "namespace doesn't exist" to reset its local replica no longer sees that during a primary restart, which is the point; one that only retried sees a different error text.
    • sqld replicas: snapshot maps UNAVAILABLE to SnapshotPending, so a replica waiting for a snapshot loops on 1 s sleeps logging "snapshot not ready yet" until the primary is back. RemoteConnection::connect in the write proxy retries UNAVAILABLE with uncapped exponential backoff, where it failed immediately on FAILED_PRECONDITION before.
  • Fault metrics change shape. http/user/trace.rs counts any HTTP 5xx in libsql_server_user_http_fault{protcol="http"} and treats gRPC FAILED_PRECONDITION as success. Restart-time errors were therefore invisible there. After this change they appear as 503 and UNAVAILABLE on both protocols, and libsql_server_user_http_response{status="503"} rises during restarts. The first rollout of this build looks like today, because the stopping pods run the old binary; the next rolling restart is the first to show the burst. Check dashboards, alerts and deploy gates that read those series before then. Replicas log a non-fatal replication error instead of namespace ... doesn't exist, trying to replicate again while their primary restarts.
  • The 503 window lasts the whole namespace teardown. The server shuts the namespace store down before it stops accepting requests, and /health keeps answering 200, so from SIGTERM until every namespace is checkpointed the pod stays in rotation and answers 503 to every request, with no Retry-After. Clients must retry 503 with backoff. Before this change the same window produced the false 404s. Locally it was 0.5 s for 2,000 loaded namespaces and 1.7 s for 5,000 (debug build); our production pods hold about a quarter of max_active_namespaces, so expect well under a second there, inside the sync service's per-call retry budget. Measure it in staging all the same.
  • Replicas with lazy creation. A replica request waiting while its namespace is taken out of the cache now retries. Replicas create namespaces lazily, so the retry passes the metastore check and reopens the namespace. For a name that was destroyed in the meantime, that opens a fresh replica namespace, as any request for an unknown name does on a replica today; the replicator then gets FAILED_PRECONDITION from the primary.
  • Eviction is restart-only in production. The pods we checked hold a quarter of max_active_namespaces, so capacity eviction does not occur; only the 24 h idle eviction does, which races a request only when one arrives as the idle timer fires. Expect libsql_server_namespace_emptied_entry to rise during restarts and stay flat otherwise.
  • Error-grouping tooling will see a new error. Clients that logged the false 404 as an application error will log the 503 the same way after the first post-fix restart, under a new message (Namespace store has shutdown). Tooling that opens issues per distinct error will open one for it once; it is the same restart-time condition, now with the right status.
  • Rollback is a revert. Nothing on disk or on the wire changes.

Not in scope (follow-ups)

Ordered by payoff. #53, stacked on this PR through #51, fixes items 2, 3 and 4, part of 5, and the metric in 8.

  1. Shutdown order with a readiness signal. Fail readiness once has_shutdown is set (a separate endpoint, since failing /health would end the pod mid-flush), stop accepting connections, drain in-flight requests with a timeout, then take the namespaces out. This turns the 503 window into zero errors and closes the last shutdown gap, a load already inside make_namespace when the flag is set.
  2. MetaStore::handle resurrects records. handle inserts a default in-memory record when the namespace is missing. A request that passes exists just before destroy removes the record re-creates it, and can then open an empty database over the directory being deleted; that is the delete-under-load race above. This predates this change; the retry loop repeats the exists/handle pair per attempt, so a request that retried has up to three such windows instead of one. A non-inserting handle_if_exists closes it. It also removes the last reason ProxyService::extract_context skips authentication for a missing namespace. Fixed in libsql-server: make namespace creation all-or-nothing (stacked on #51) #53: with looks a namespace up with a non-inserting existing_handle.
  3. Entries left empty. A fork that fails before storing its record leaves an empty entry cached. with then gives up with NamespaceUnavailable on every request for that name, and create of that name returns Ok without loading (so a dump is dropped), until the entry is evicted or the server restarts. On the base the same sequence answers 404. Until the fix ships, a sustained gave_up rate for one namespace means a restart. Fixed in libsql-server: make namespace creation all-or-nothing (stacked on #51) #53: load_namespace drops a leftover empty entry and reloads, under moka's key lock (evict_if_still_empty, folded in from the closed libsql-server: drop a namespace cache entry that was left empty #59), and create's reservation fills such an entry under its lock, so a dump is applied.
  4. Fork onto an existing but unloaded namespace destroys it. Found by the matrix, identical on the base. fork tests to_lock.is_some() instead of the metastore, so an unloaded destination passes. The rename fails with Directory not empty, ForkTask::fork runs remove_dir_all on the destination directory on any error, and the store's cleanup removes the record. Observed: 500, then the destination answers 404 and its directory is gone. Admin API only, but data loss. Fix: check metadata.exists(&to) before get_with(to), and make ForkTask::fork remove only a directory it created. Deserves its own PR soon. Fixed in libsql-server: make namespace creation all-or-nothing (stacked on #51) #53: fork reserves its destination, and the reservation rejects a name the metastore knows. The matrix shows 400 already exists with the destination intact. The ForkTask::fork cleanup that removes any directory on error remains.
  5. Cache entry state. Arc<RwLock<Option<Namespace>>> lets "empty" be a normal state that every reader must handle; with, fork, create and reset each handle it differently. libsql-server: make namespace creation all-or-nothing (stacked on #51) #53 makes the operations that hold an entry locked safe (it pins them in in_flight, its eviction listener does not wait on a pinned entry, and install re-inserts a namespace whose entry was evicted). It still uses the same entry type, though, and defers the Filling | Loaded | Gone state with the reservation held outside the cache.
  6. Replicator backoff. After a namespace is destroyed on the primary, the replica retries NamespaceDoesntExist in a tight loop: about 5,500 log lines in 2 s per namespace (the existing TODO in configurator/replica.rs).
  7. with on the hot path. exists and handle both lock the global metastore mutex, and handle builds a MetaStoreHandle that a cache hit discards. Check the cache first and call handle only on a miss. Benchmark before and after.
  8. Smaller items. libsql_server_namespace_load_latency is recorded on every lookup, cache hits included, so it counts requests, not loads (libsql-server: make namespace creation all-or-nothing (stacked on #51) #53 records it only for real loads). checkpoint of an unknown or unloaded namespace returns Ok (pre-existing). Under eviction churn, Namespace::shutdown logs unable to remove .sentinel file when a namespace was reopened while the evicted instance was still shutting down (pre-existing, both binaries). A request that holds a connection maker when destroy removes the directory fails with 500 unable to open database file. A proxied write in flight when the primary exits answers 500 PrimaryStreamDisconnect, and new ones wait in the write proxy's unbounded Unavailable retry loop until the primary is back. format_err logs every 503 at error level, as it did the 404s.
  9. Test debt. A lazy-creation (replica) fixture: under lazy creation with skips the metastore check and never answers DoesntExist, and a request waiting while its namespace is destroyed reopens a fresh replica namespace for the name; the reload, give-up and destroy tests should run with it. Counters asserted through a DebuggingRecorder. A positive control for checkpoint. start_waiting/settle depend on moka's reference counts; a #[cfg(test)] hook in with would decouple them. One shared make_store for the store and scheduler tests.

NamespaceStore::with mapped an emptied cache entry to NamespaceDoesntExist.
Entries are emptied by shutdown (which runs while HTTP and gRPC services
still accept requests), by the cache's eviction listener, and by destroy. A
request that got the entry before it was emptied answered 404 "namespace
doesn't exist" for a namespace the metastore still has. Clients cannot tell
that from a namespace that is really gone, and a 404 is the one answer a
client may act on as authoritative (for example by creating the namespace
again or discarding local state).

with now:
- answers NamespaceStoreShutdown (503) once the store is shut down, on every
  path, so a request after shutdown no longer reloads the namespace;
- looks the namespace up again when the entry it got was emptied, so an
  evicted namespace is reloaded instead of reported missing;
- drops an emptied entry that is still the cached one, under moka's key lock
  and only if it is still cached, empty and unlocked, so the next lookup
  loads the namespace; it never fills an entry itself;
- answers NamespaceDoesntExist only when the metastore no longer has the
  namespace, and the new NamespaceUnavailable (503) otherwise.

Also:
- fork loads its source through with. create refuses once the store is shut
  down, and drops a left-empty entry before loading, so its restore option is
  applied. checkpoint no longer caches an empty entry for a namespace that is
  not loaded.
- gRPC: rpc::namespace_error_status maps both 503 errors to UNAVAILABLE in
  replication auth, ReplicationLogService and ProxyService, instead of
  INTERNAL, and neither auth path falls back to another strategy for them.
- libsql_server_namespace_emptied_entry counts emptied entries a request
  meets; looking again, dropping a left-empty entry and giving up are logged
  at warn.

Related upstream issue: tursodatabase#1772.
@tszymczyszyn-shopify
tszymczyszyn-shopify force-pushed the tszymczyszyn/no-false-namespace-not-found branch from 4adb7a4 to 2d1c5aa Compare October 14, 2026 20:06
No test reached the final `Err` of `with`, or the `else` in `fork` that
answers for a source emptied between its two loads. Every test exited the
loop earlier: from the shutdown check, from the metastore check, or with
`Ok`. Putting the original `NamespaceDoesntExist` back on either path
passed every test.

Split the loop into `with_attempts(namespace, max_attempts, f)`; `with`
passes `MAX_EMPTIED_ENTRY_ATTEMPTS`. With one attempt, the emptied entry a
request finds is the last one it looks at, so the tail is the only exit.
Two tests pin it: `NamespaceUnavailable` while the metastore still has the
namespace, `NamespaceDoesntExist` once it does not.

For `fork`, holding the destination entry's write lock stops it between
loading the source through `with` and reading it from its own lookup.
Emptying the source there reaches the `else`, which must answer
`NamespaceUnavailable`.

`start_waiting_request` is now a thin wrapper over `start_waiting`, which
takes the future to run, so these tests reuse the same rendezvous.
For an entry that was left empty, `with` used all three attempts: one to
find it empty, one to recognise it as still cached and drop it, one to
load the namespace. There was no attempt left for the fresh entry, so if
that was emptied as well (the cache rejecting a new entry at capacity, for
example) the request got a 503 that one more lookup would have avoided.

Count only lookups that found an emptied entry. The drop branch clears
`emptied`, so it cannot run twice without such a lookup in between, and
the loop still ends after at most twice the budget.

The new test gives `with_attempts` a budget of two over a left-empty
entry. With the old counting the budget was gone after the drop.
`with`, `create` and `fork` check `has_shutdown` before they load, but
the load itself awaits the metastore and then opens the namespace. A
request that passed the check just before shutdown started could open a
namespace after `shutdown()` had taken out every cached entry. Nothing
would ever shut that namespace down. `reset`, which only replicas call,
had no check at all and fills an entry of its own.

Check the flag again inside `load_namespace`'s init, which only runs on a
cache miss, so one check covers every caller. The error comes back as
`Error::Ref`, which the HTTP and gRPC mappings and both auth paths unwrap
with `as_ref`. Add the same check at the top of `reset`.

A load that was already opening the namespace when shutdown started can
still finish. Closing that needs the shutdown-ordering follow-up: stop
accepting requests, drain, then take the namespaces out.
`ReplicaProxyService::do_auth` sent `NamespaceStoreShutdown` and
`NamespaceUnavailable` to its generic arm, which answers `INTERNAL`.
Before, a replica shutting down produced a false `NamespaceDoesntExist`
there, and the request fell back to the server-wide auth strategy. So
this change had altered what clients of a restarting replica see, from a
wrong fallback to an error they do not retry, while primaries answer
`UNAVAILABLE` for the same condition.

Map the two errors through `namespace_error_status`, as the primary's
proxy and replication services do. Neither falls back to the user
strategy.
`libsql_server_namespace_emptied_entry` rises on every deploy, because
every request that was waiting while shutdown took its namespace out
looks again and succeeds or gets a 503 from the shutdown check. The two
outcomes an operator would alert on had no metric: a request answered
with an error because every lookup found an emptied entry, and a cache
entry dropped because something left it empty. Both were log lines only.

Add `libsql_server_namespace_emptied_entry_gave_up`, incremented where
`with` gives up, and `libsql_server_namespace_left_empty_entry_dropped`,
incremented when `evict_if_still_empty` removed the entry (a `Nop` is not
counted). Neither has a label.
The test for a namespace destroyed while a request waited removed the
metastore record and emptied the entry by hand. The contract it checks,
that the request answers "missing" because the record is gone, depends
on `destroy` removing the record before it takes the namespace out.
Swapping those two steps in `destroy` failed no test.

Run the real `destroy` in a task. It removes the record, then waits for
the entry's write lock the test holds. The test waits for the record to
go, then takes the namespace out itself and drops the lock, so the
request finds the entry empty whichever of the two the lock wakes first.
Letting `destroy` take the namespace instead would make the outcome
depend on that order. With the steps swapped in `destroy`, the wait for
the record times out.
The eviction test and both fork tests only compared the namespace's
name, which any instance has, including one opened outside the cache or
one filled into the emptied entry. Only the left-empty test checked the
cache afterwards.

After each reload, assert that the cache holds a different entry, that
it holds a namespace, and that the emptied entry is still empty.
Every test had one request on the emptied entry, so `evict_if_still_empty`
ran alone and in a fixed order. A rolling restart or an eviction empties
an entry under all the requests waiting on it at once, and the identity
and lock checks exist for that case.

Eight requests wait on one entry that is then left empty. Each must
succeed, and all of them must be served by the one namespace the cache
holds afterwards, compared by the pointer of its `Stats`. The test runs
on the multi-thread runtime so the requests really interleave. 50
consecutive runs passed.
- The doc comment on `evict_if_still_empty` promised that it "cannot open
  a namespace that the cache no longer holds". It cannot see a `fork` or
  `reset` that holds the entry but has not locked it yet, and moka's
  `Op::Remove` removes whatever the cache holds when it runs; its compute
  lock does not exclude `try_get_with` or `invalidate`. Say so, and log an
  error when the removed entry is not the one the closure saw.
- The retry log fires once per request that was waiting when shutdown or
  eviction took the namespace. That is expected and the counter carries
  the signal; log it at debug. The drop and give-up logs stay at warn.
- The eviction listener logged "deallocated" before checking whether the
  entry held anything, so dropping a left-empty entry logged a
  deallocation that did not happen.
- Note the known hazard in `fork` and `reset`: an entry write lock held
  across a cache call, which moka's inline maintenance can wait for. The
  fix (reserving the name outside the cache) is a follow-up.
`ProxyService::extract_context` authenticates a request for a namespace
reported as missing from `x-proxy-authorization`, a header the client
writes. Before this change, a false "missing" during shutdown or eviction
let a forged `FullAccess` through for a namespace that has its own keys.

The test for the 503 answer sent no header, so a change that brought the
fallback back for the two 503 errors would have passed it as long as the
request then failed for some other reason. Send the header: with the
fallback, `extract_context` returns `Ok`, and the test fails.
- `settle`, `wait_until` and `wait_until_async` spun on `yield_now` for up
  to ten seconds. That burns a core while the awaited event runs on a
  blocking thread, as `destroy`'s metastore removal does. Sleep a
  millisecond between checks instead.
- The fork tests ran on the current-thread runtime. `fork`'s cleanup on
  failure calls `block_in_place`, which panics there, so a regression in
  `fork` would have surfaced as that panic rather than the real error.
  Run them on the multi-thread runtime.
- Note that the reference counting in `settle` and `start_waiting` is
  what moka 0.12.8 does, and that an upgrade fails these tests with a
  timeout rather than hanging them.
The loop in `with` tracked the emptied entry it had found so that, when the
next lookup returned the same entry, it could drop that entry from the
cache (`evict_if_still_empty`) and reload the namespace. That handled an
entry that something left cached but empty, which after the `checkpoint`
fix only a failed fork does, and it was the most delicate code in this
change: a compute under moka's key lock, with windows it could not close
on its own. It is removed here and kept for a separate change, to be
reconciled with the cache entry redesign.

What remains is a plain loop: look the namespace up, serve it if its entry
holds it, otherwise count the emptied entry and look again, up to three
times. An entry that stays empty is found on every attempt, so the request
gives up with `NamespaceUnavailable` and the namespace answers 503 until
the entry is evicted or the server restarts, as
`with_gives_up_with_unavailable_when_the_entry_stays_empty` pins. The
give-up counter now records only that outcome: giving up because the store
shut down, or because the namespace is really gone, is not a failure of
the store.

Since the give-up path is reachable through `with` itself now, the
`with_attempts` test seam and the `left_empty_entry_dropped` counter go
too. The fork test that said "emptied while waiting" left the entry
cached, which is the stays-empty case; it now evicts the entry as the
`with` test does.

Also corrects the comment in `reset`: `make_namespace` does reach the
cache on a replica, through `with` for a shared schema during the
replication handshake. That hazard predates this change.
`create` stores the namespace's record, then loads the namespace. Since
`load_namespace` refuses to open a namespace once the store has shut
down, a shutdown that starts between the two left the record behind with
nothing loaded. Every retry then answered "already exists", and the
restore option (a dump, say) was never applied. Before this change the
same window loaded the namespace, into an entry nothing would shut down.

The refusal happens before anything is opened or written, so `create`
now removes the record again when its load is refused for that reason.
Other load failures are left as they were.
The three gRPC authentication paths each matched the store's error to
decide between "missing namespace" (fall back to the user strategy, or
skip authentication) and "exists but cannot be served" (UNAVAILABLE, no
fallback). Each unwrapped `Error::Ref` on its own, and the replica proxy
did not, so a wrapped `NamespaceDoesntExist` would have been INTERNAL
there. They now decide on the code `namespace_error_status` returns,
which is the one place that classifies these errors and the one that is
tested with wrapped errors.

`Error::as_ref` unwraps nested `Ref`s, as `into_response` already did.

The Hello test for a shut-down store now configures Basic auth and sends
no credentials, so that it fails if the UNAVAILABLE case ever falls back
to the user strategy.
`with_gives_up_with_unavailable_when_the_entry_stays_empty` becomes
`with_reports_unavailable_not_missing_for_an_entry_left_empty`: the
same form as its neighbours, the PR's term for the case, and the contract
it pins (a namespace that exists must not be reported missing).
…ever read like 404s

Eight requests waiting on one entry that is then evicted all look the
namespace up again at once; `try_get_with` coalesces them into one load
and one fresh entry. Before, nothing asserted that on the eviction path.

Clients tell a missing namespace from a temporarily unavailable one by
the status, and some also by the message. The two 503 messages must not
contain "doesn't exist" or "not found".
tszymczyszyn-shopify added a commit that referenced this pull request Oct 16, 2026
`NamespaceStore::create` persisted the namespace config before setting the
namespace up, and nothing undid it: a dump that failed to import left a
"ghost" namespace (config without data) whose name could not be reused and
which the next user request lazily turned into an empty database. A request
cancelled mid-import ran no cleanup at all, since the error branch of the
future is never reached when it is dropped, and the schema configurator had
no cleanup even for errors.

The metastore write is now the commit point and everything before it is
reversible. Contract for callers: 2xx means complete; anything else,
including a lost connection, means retry; `already exists` on the retry
means the earlier attempt did complete.

Reservation (store). `create` and `fork` write-lock the name's cache entry
for the duration and keep the config in memory only. `publish` installs the
finished namespace; `abort` (awaited on error paths) and `Drop` (the
creating future was dropped, i.e. the request was cancelled) undo the
attempt — config, then directory, then unpin — and only then release the
lock, so waiters observe a namespace that does not exist, never a half-built
one, and a retry issued the moment the error arrives neither races the
cleanup nor finds leftovers. Replaces fork's ad-hoc guard, which blocked in
place (panics on a current-thread runtime) and published before flushing.

Marker and tombstones (namespace/incomplete.rs). A new namespace directory
carries `.incomplete` (fsynced, so it is durable before anything else in the
directory can be) until the config is persisted; the store removes it after
publishing. A creation that finds a marked directory for a name the
metastore does not know discards it, so a crash mid-import never blocks a
retry; a startup sweep does the same and deletes leftover tombstones; the
metastore's filesystem recovery skips marked directories. Directories are
never deleted in place: `discard_dir` renames them into `<base>/.discarded/`
(instant; frees the path; a still-running import executor writes into the
tombstone) and deletes in the background. `FreshDir` (primary and schema
configurators) creates the marked directory synchronously, guard first, and
discards it on drop. Fork marks its temporary directory, and only removes
its destination on failure if it carries the marker.

Metastore. `discard` (via the change queue, so it is ordered after a config
store still queued when the request was cancelled) forgets a namespace
unconditionally — `remove`'s linked-DB guards would otherwise leave a ghost
when a linked-DB creation's flush fails. `existing_handle` looks a namespace
up without registering it; `with()` and the fork source use it, and the lazy
init re-checks existence, so a lookup racing the end of a failed creation
cannot resurrect the name. `process`'s flush branch no longer holds
`configs` across `try_process` (lock-order inversion against `remove`).

Cache eviction. The reservation is the entry's write lock, which moka knows
nothing about. If the cache evicted that entry during a long operation
(capacity pressure): a concurrent lookup lazily opened the directory being
written; the eviction listener awaited the lock and moka awaits async
listeners inline from whichever cache op runs maintenance, so that op hung
for the whole import and, once the write-op channel filled, so did every
namespace load; an operation touching the cache while holding its own entry
(fork) could deadlock. Now the locked entry is pinned in `in_flight`;
`load_namespace` and `destroy` consult it first and wait on it; the listener
spawns its shutdown only for pinned entries; `install` publishes into a
fresh entry when the locked one was evicted, or hands the namespace to the
old entry when the name was destroyed meanwhile (destroy is waiting on it).
`shutdown` skips pinned entries; `create`/`fork` re-check `has_shutdown`
before the commit.

Empty cache entries. A rejected `acquire` (`already exists` for a namespace
that is not loaded, e.g. a retry after a restart) left behind the empty
entry its `get_with` inserted, and `try_get_with` returned that entry forever
instead of loading the namespace. `acquire` removes the entry it created;
`load_namespace` treats an empty, unlocked entry as a miss; `checkpoint` no
longer inserts one for an unloaded namespace (the pre-existing source).

Single-namespace mode: `create(default, Latest)` stays a config upsert;
creating the default namespace from a dump while it exists is rejected
instead of silently skipping the import.

Needs async-lock 3 for `RwLock::write_arc` (already in the lockfile; the
bump removes the duplicate 2.x). Dump tests set `tcp_capacity(4096)`: the
fsync makes setup's following blocking fs ops slower in real time, turmoil
advances simulated time while a host waits on them, and the dump store
writes 1-byte chunks past turmoil's default 64-segment socket buffer.

Tests. Store unit tests build a real `NamespaceStore` with a channel-fed
streaming import: lookup waits across an eviction and sees the data, or
"doesn't exist" when the import fails; destroy during creation; retry the
instant the error returns; abort mid-import; discard vs remove's guards.
Unit tests for the marker protocol, tombstones, sweep and metastore
recovery. Integration (turmoil): failed creation leaves no trace (both
importers, schema configurator), cancelled creation (cancel fires once the
marker exists), concurrent requests wait, crash remnants discarded, unmarked
directories untouched, marker on a published namespace ignored (create
before any lookup after a restart), single-namespace dump create rejected.

Rebased onto the false-404 fix (#56), whose semantics `with` and
`load_namespace` keep: `with` checks for shutdown on every attempt,
looks the namespace up again (up to three times) when the entry it got
was emptied, and answers `NamespaceDoesntExist` only when the metastore
has no record (otherwise `NamespaceUnavailable` or
`NamespaceStoreShutdown`, both 503); `load_namespace` refuses to open a
namespace once the store has shut down. `with` keeps
`existing_handle`, so the retry cannot resurrect a destroyed record.

An empty entry left in the cache is dropped with `evict_if_still_empty`
(from #59): under moka's key lock, only if it is still the cached entry,
still empty and unlocked. That replaces the `get`-then-`invalidate`
pairs in `load_namespace` and `Reservation::acquire`, which could evict
a fresh entry loaded in between. Dropped entries are counted in
`libsql_server_namespace_left_empty_entry_dropped`.
@tszymczyszyn-shopify
tszymczyszyn-shopify removed this pull request from stack #60 October 16, 2026 11:35
@tszymczyszyn-shopify
tszymczyszyn-shopify added this pull request to stack #61 October 16, 2026 11:35
tszymczyszyn-shopify added a commit that referenced this pull request Oct 16, 2026
The design doc described how the reader hands statements to the executor,
but not why the queue is bounded or why it has both a message limit and a
byte limit. Add that to the architecture section. Replace the "< 32 MiB"
estimate in the memory model with the values measured on staging, and
name the build the staging run used now that the stack sits on #56.
tszymczyszyn-shopify added a commit that referenced this pull request Oct 16, 2026
`NamespaceStore::create` persisted the namespace config before setting the
namespace up, and nothing undid it: a dump that failed to import left a
"ghost" namespace (config without data) whose name could not be reused and
which the next user request lazily turned into an empty database. A request
cancelled mid-import ran no cleanup at all, since the error branch of the
future is never reached when it is dropped, and the schema configurator had
no cleanup even for errors.

The metastore write is now the commit point and everything before it is
reversible. Contract for callers: 2xx means complete; anything else,
including a lost connection, means retry; `already exists` on the retry
means the earlier attempt did complete.

Reservation (store). `create` and `fork` write-lock the name's cache entry
for the duration and keep the config in memory only. `publish` installs the
finished namespace; `abort` (awaited on error paths) and `Drop` (the
creating future was dropped, i.e. the request was cancelled) undo the
attempt — config, then directory, then unpin — and only then release the
lock, so waiters observe a namespace that does not exist, never a half-built
one, and a retry issued the moment the error arrives neither races the
cleanup nor finds leftovers. Replaces fork's ad-hoc guard, which blocked in
place (panics on a current-thread runtime) and published before flushing.

Marker and tombstones (namespace/incomplete.rs). A new namespace directory
carries `.incomplete` (fsynced, so it is durable before anything else in the
directory can be) until the config is persisted; the store removes it after
publishing. A creation that finds a marked directory for a name the
metastore does not know discards it, so a crash mid-import never blocks a
retry; a startup sweep does the same and deletes leftover tombstones; the
metastore's filesystem recovery skips marked directories. Directories are
never deleted in place: `discard_dir` renames them into `<base>/.discarded/`
(instant; frees the path; a still-running import executor writes into the
tombstone) and deletes in the background. `FreshDir` (primary and schema
configurators) creates the marked directory synchronously, guard first, and
discards it on drop. Fork marks its temporary directory, and only removes
its destination on failure if it carries the marker.

Metastore. `discard` (via the change queue, so it is ordered after a config
store still queued when the request was cancelled) forgets a namespace
unconditionally — `remove`'s linked-DB guards would otherwise leave a ghost
when a linked-DB creation's flush fails. `existing_handle` looks a namespace
up without registering it; `with()` and the fork source use it, and the lazy
init re-checks existence, so a lookup racing the end of a failed creation
cannot resurrect the name. `process`'s flush branch no longer holds
`configs` across `try_process` (lock-order inversion against `remove`).

Cache eviction. The reservation is the entry's write lock, which moka knows
nothing about. If the cache evicted that entry during a long operation
(capacity pressure): a concurrent lookup lazily opened the directory being
written; the eviction listener awaited the lock and moka awaits async
listeners inline from whichever cache op runs maintenance, so that op hung
for the whole import and, once the write-op channel filled, so did every
namespace load; an operation touching the cache while holding its own entry
(fork) could deadlock. Now the locked entry is pinned in `in_flight`;
`load_namespace` and `destroy` consult it first and wait on it; the listener
spawns its shutdown only for pinned entries; `install` publishes into a
fresh entry when the locked one was evicted, or hands the namespace to the
old entry when the name was destroyed meanwhile (destroy is waiting on it).
`shutdown` skips pinned entries; `create`/`fork` re-check `has_shutdown`
before the commit.

Empty cache entries. A rejected `acquire` (`already exists` for a namespace
that is not loaded, e.g. a retry after a restart) left behind the empty
entry its `get_with` inserted, and `try_get_with` returned that entry forever
instead of loading the namespace. `acquire` removes the entry it created;
`load_namespace` treats an empty, unlocked entry as a miss; `checkpoint` no
longer inserts one for an unloaded namespace (the pre-existing source).

Single-namespace mode: `create(default, Latest)` stays a config upsert;
creating the default namespace from a dump while it exists is rejected
instead of silently skipping the import.

Needs async-lock 3 for `RwLock::write_arc` (already in the lockfile; the
bump removes the duplicate 2.x). Dump tests set `tcp_capacity(4096)`: the
fsync makes setup's following blocking fs ops slower in real time, turmoil
advances simulated time while a host waits on them, and the dump store
writes 1-byte chunks past turmoil's default 64-segment socket buffer.

Tests. Store unit tests build a real `NamespaceStore` with a channel-fed
streaming import: lookup waits across an eviction and sees the data, or
"doesn't exist" when the import fails; destroy during creation; retry the
instant the error returns; abort mid-import; discard vs remove's guards.
Unit tests for the marker protocol, tombstones, sweep and metastore
recovery. Integration (turmoil): failed creation leaves no trace (both
importers, schema configurator), cancelled creation (cancel fires once the
marker exists), concurrent requests wait, crash remnants discarded, unmarked
directories untouched, marker on a published namespace ignored (create
before any lookup after a restart), single-namespace dump create rejected.

Rebased onto the false-404 fix (#56), whose semantics `with` and
`load_namespace` keep: `with` checks for shutdown on every attempt,
looks the namespace up again (up to three times) when the entry it got
was emptied, and answers `NamespaceDoesntExist` only when the metastore
has no record (otherwise `NamespaceUnavailable` or
`NamespaceStoreShutdown`, both 503); `load_namespace` refuses to open a
namespace once the store has shut down. `with` keeps
`existing_handle`, so the retry cannot resurrect a destroyed record.

An empty entry left in the cache is dropped with `evict_if_still_empty`
(from #59): under moka's key lock, only if it is still the cached entry,
still empty and unlocked. That replaces the `get`-then-`invalidate`
pairs in `load_namespace` and `Reservation::acquire`, which could evict
a fresh entry loaded in between. Dropped entries are counted in
`libsql_server_namespace_left_empty_entry_dropped`.
tszymczyszyn-shopify added a commit that referenced this pull request Oct 16, 2026
`NamespaceStore::create` persisted the namespace config before setting the
namespace up, and nothing undid it: a dump that failed to import left a
"ghost" namespace (config without data) whose name could not be reused and
which the next user request lazily turned into an empty database. A request
cancelled mid-import ran no cleanup at all, since the error branch of the
future is never reached when it is dropped, and the schema configurator had
no cleanup even for errors.

The metastore write is now the commit point and everything before it is
reversible. Contract for callers: 2xx means complete; anything else,
including a lost connection, means retry; `already exists` on the retry
means the earlier attempt did complete.

Reservation (store). `create` and `fork` write-lock the name's cache entry
for the duration and keep the config in memory only. `publish` installs the
finished namespace; `abort` (awaited on error paths) and `Drop` (the
creating future was dropped, i.e. the request was cancelled) undo the
attempt — config, then directory, then unpin — and only then release the
lock, so waiters observe a namespace that does not exist, never a half-built
one, and a retry issued the moment the error arrives neither races the
cleanup nor finds leftovers. Replaces fork's ad-hoc guard, which blocked in
place (panics on a current-thread runtime) and published before flushing.

Marker and tombstones (namespace/incomplete.rs). A new namespace directory
carries `.incomplete` (fsynced, so it is durable before anything else in the
directory can be) until the config is persisted; the store removes it after
publishing. A creation that finds a marked directory for a name the
metastore does not know discards it, so a crash mid-import never blocks a
retry; a startup sweep does the same and deletes leftover tombstones; the
metastore's filesystem recovery skips marked directories. Directories are
never deleted in place: `discard_dir` renames them into `<base>/.discarded/`
(instant; frees the path; a still-running import executor writes into the
tombstone) and deletes in the background. `FreshDir` (primary and schema
configurators) creates the marked directory synchronously, guard first, and
discards it on drop. Fork marks its temporary directory, and only removes
its destination on failure if it carries the marker.

Metastore. `discard` (via the change queue, so it is ordered after a config
store still queued when the request was cancelled) forgets a namespace
unconditionally — `remove`'s linked-DB guards would otherwise leave a ghost
when a linked-DB creation's flush fails. `existing_handle` looks a namespace
up without registering it; `with()` and the fork source use it, and the lazy
init re-checks existence, so a lookup racing the end of a failed creation
cannot resurrect the name. `process`'s flush branch no longer holds
`configs` across `try_process` (lock-order inversion against `remove`).

Cache eviction. The reservation is the entry's write lock, which moka knows
nothing about. If the cache evicted that entry during a long operation
(capacity pressure): a concurrent lookup lazily opened the directory being
written; the eviction listener awaited the lock and moka awaits async
listeners inline from whichever cache op runs maintenance, so that op hung
for the whole import and, once the write-op channel filled, so did every
namespace load; an operation touching the cache while holding its own entry
(fork) could deadlock. Now the locked entry is pinned in `in_flight`;
`load_namespace` and `destroy` consult it first and wait on it; the listener
spawns its shutdown only for pinned entries; `install` publishes into a
fresh entry when the locked one was evicted, or hands the namespace to the
old entry when the name was destroyed meanwhile (destroy is waiting on it).
`shutdown` skips pinned entries; `create`/`fork` re-check `has_shutdown`
before the commit.

Empty cache entries. A rejected `acquire` (`already exists` for a namespace
that is not loaded, e.g. a retry after a restart) left behind the empty
entry its `get_with` inserted, and `try_get_with` returned that entry forever
instead of loading the namespace. `acquire` removes the entry it created;
`load_namespace` treats an empty, unlocked entry as a miss; `checkpoint` no
longer inserts one for an unloaded namespace (the pre-existing source).

Single-namespace mode: `create(default, Latest)` stays a config upsert;
creating the default namespace from a dump while it exists is rejected
instead of silently skipping the import.

Needs async-lock 3 for `RwLock::write_arc` (already in the lockfile; the
bump removes the duplicate 2.x). Dump tests set `tcp_capacity(4096)`: the
fsync makes setup's following blocking fs ops slower in real time, turmoil
advances simulated time while a host waits on them, and the dump store
writes 1-byte chunks past turmoil's default 64-segment socket buffer.

Tests. Store unit tests build a real `NamespaceStore` with a channel-fed
streaming import: lookup waits across an eviction and sees the data, or
"doesn't exist" when the import fails; destroy during creation; retry the
instant the error returns; abort mid-import; discard vs remove's guards.
Unit tests for the marker protocol, tombstones, sweep and metastore
recovery. Integration (turmoil): failed creation leaves no trace (both
importers, schema configurator), cancelled creation (cancel fires once the
marker exists), concurrent requests wait, crash remnants discarded, unmarked
directories untouched, marker on a published namespace ignored (create
before any lookup after a restart), single-namespace dump create rejected.

Rebased onto the false-404 fix (#56), whose semantics `with` and
`load_namespace` keep: `with` checks for shutdown on every attempt,
looks the namespace up again (up to three times) when the entry it got
was emptied, and answers `NamespaceDoesntExist` only when the metastore
has no record (otherwise `NamespaceUnavailable` or
`NamespaceStoreShutdown`, both 503); `load_namespace` refuses to open a
namespace once the store has shut down. `with` keeps
`existing_handle`, so the retry cannot resurrect a destroyed record.

An empty entry left in the cache is dropped with `evict_if_still_empty`
(from #59): under moka's key lock, only if it is still the cached entry,
still empty and unlocked. That replaces the `get`-then-`invalidate`
pairs in `load_namespace` and `Reservation::acquire`, which could evict
a fresh entry loaded in between. Dropped entries are counted in
`libsql_server_namespace_left_empty_entry_dropped`.

@sle-c sle-c left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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