Repository navigation
libsql-server: make namespace creation all-or-nothing (stacked on #51) - #53
Draft
tszymczyszyn-shopify wants to merge 2 commits into
Draft
tszymczyszyn-shopify wants to merge 2 commits into
tszymczyszyn-shopify wants to merge 2 commits into
Conversation
`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: - `Reservation` (store) write-locks the namespace's cache entry for the duration of the creation and keeps the config in memory only. `publish` installs the finished namespace; dropping the reservation unpublished (error or cancellation) removes the in-memory config and only then releases the lock, so waiters observe a namespace that does not exist, never a partial one. `fork` uses it too, replacing its ad-hoc guard, which blocked in place (panics on a current-thread runtime) and published before flushing. - `FreshDir` (configurators) removes a brand-new namespace directory when setup fails or its future is dropped; used by the primary and schema configurators. - An `.incomplete` marker is written into a new directory and removed once the config is persisted. A creation that finds a marked directory for a name the metastore doesn't know discards it, so a process crash during an import never blocks a retry; the metastore's filesystem recovery skips marked directories. Directories without the marker are left alone. Requests for a namespace that is being created wait for the outcome, as during `fork` and `reset`. In single-namespace mode `create` of the default namespace remains a config upsert, but creating it *from a dump* while it exists is now rejected instead of silently skipping the import. 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. Requires async-lock 3 for `RwLock::write_arc` (already in the lockfile; the bump removes the duplicate 2.x version). Tests: 9 lifecycle tests (failed/cancelled creation leaves no trace for both importers and for shared-schema namespaces, concurrent requests wait, crash remnants are discarded, unmarked directories and published namespaces are untouched, single-namespace dump create is rejected) plus unit tests for the guards. Nine of the eleven integration tests fail on the previous code.
Adds docs/ATOMIC_NAMESPACE_CREATE_DESIGN.md, states the all-or-nothing creation contract in ADMIN_API.md, and marks the lifecycle items of the streaming dump import design (section 13) as fixed.
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
Stacked on #51 (targets
tszymczyszyn/streaming-dump-importer; will be retargeted tov0.9.30-shopify-patchesonce #51 merges — only the last two commits are this PR).Makes
POST /v1/namespaces/:ns/createall-or-nothing. Fixes the pre-existing lifecycle gap called out in #51's design doc §13: a dump that failed to import left a ghost namespace (config without data, name unusable, lazily turned into an empty DB on first access); a request cancelled mid-import ran no cleanup at all; the schema configurator had no cleanup even on error.Design:
docs/ATOMIC_NAMESPACE_CREATE_DESIGN.md.Contract (new, documented in
ADMIN_API.md)After a non-2xx or a lost connection the namespace is either absent (no config, no directory, name reusable) or complete — never partial. For callers:
2xx⇒ complete; anything else ⇒ retry;already existson the retry ⇒ the earlier attempt did complete.How
The metastore write becomes the commit point; everything before it is reversible.
Reservationgeneralises the patternforkalready used (write-lock the cache entry, config in memory only, flush on success). Dropping it unpublished — error or cancellation — removes the config on the blocking pool and releases the lock only afterwards, so waiters see "doesn't exist", never a half-built namespace.forknow uses it too; its old guard usedblock_in_place(panics on a current-thread runtime) and published before flushing.FreshDirremoves a brand-new directory on drop unlesskeep()-ed; primary and schema configurators..incompletemarker for crashes: written with the directory, removed after the flush; consulted in exactly one place (create, for a name with no config and nothing loaded) so it can never wipe a published namespace;maybe_recover_from_fsskips marked dirs. Marker-less directories keep today's semantics.fork/reset).async-lock = "3"forRwLock::write_arc— already in the lockfile; the bump removes the duplicate 2.x.Behaviour changes visible to clients
GET …/configis 404 and user requests get "namespace doesn't exist" (was: 200 / "no such table").create defaultfrom a dump while it exists ⇒already exists(was: 200 with the dump silently skipped). The config-upsert behaviour of a plaincreate defaultis unchanged.Tests
FreshDir(incl. drop of the enclosing future) anddiscard_incomplete.libsql-serversuite under nextest: identical results to the base branch (local_sync_with_writesand the wall-clock-ratioreplica_no_resync_on_restartfail under parallel load on both; both pass alone).Known limitation (inherited, documented)
If moka evicts the locked cache entry during a multi-minute import, a concurrent request can lazily open the directory being written.
reset/forkhave the same exposure today; the fix is an in-flight-operations map outside the cache — separate change.