diff --git a/docs/en/antalya/cas/architecture/mounts-and-leases.md b/docs/en/antalya/cas/architecture/mounts-and-leases.md index 74e5b521cea11..03bbab6e69a50 100644 --- a/docs/en/antalya/cas/architecture/mounts-and-leases.md +++ b/docs/en/antalya/cas/architecture/mounts-and-leases.md @@ -20,8 +20,8 @@ otherwise regenerate the same `ServerUUID` from a wiped local state directory, w silently steal an existing identity. Validation (`validateServerRootId`, `Pool/CasServerRoot.h`) is fail-closed `BAD_ARGUMENTS`, no -sanitizing fallback: non-empty, at most 255 bytes, no empty/`.`/`..` path segment, no `_files` or -`_manifests` segment. +sanitizing fallback: non-empty, at most 255 bytes, no `/`, not `.` or `..`, not `_files` or +`_manifests`. It roots four subtrees and owns catalog names at or below ``: @@ -38,6 +38,11 @@ dedup work. Ref/namespace keys are also deliberately opaque and do not embed the Each replica sharing a backend endpoint must use a distinct `cas_server_root_id`; omitting the setting is a startup error. +The id may not contain `/`, and this holds for an id that already owns its owner object too: decommission +and the staging sweep select a member's objects by key prefix, so a member `a/b` would be drained by a +decommission of `a`. A server whose id contains `/` refuses to open the disk; see +[recreating a member whose id contains `/`](/antalya/cas/operations/migration#recreate-id-with-slash). + ## The owner claim {#owner-claim} `claimOwnerOrThrow` binds `server_root_id` ↔ `server_uuid` **permanently**. The owner object is never deleted @@ -46,10 +51,10 @@ and never reassigned — decommission only tombstones it in place. | Observed at `gc/server-roots//owner` | Action | |---|---| | present, same `server_uuid`, not tombstoned | proceed | -| present, `retired_at_ms` set | `CORRUPTED_DATA` — explicitly decommissioned, refuses to resume | -| present, different `server_uuid` | `CORRUPTED_DATA` — names the regenerated-uuid-file cause | -| absent, subtree provably empty | `putIfAbsent` the owner (claim) | +| present, `retired_at_ms` set | `CORRUPTED_DATA` — explicitly decommissioned; the refusal says to configure a new `cas_server_root_id` | +| present, different `server_uuid` | `CORRUPTED_DATA` — names the regenerated-uuid-file cause and, for a dead owner, `SYSTEM CAS DROP POOL MEMBER` | | absent, subtree non-empty | `CORRUPTED_DATA` — identity lost over existing data | +| absent, subtree provably empty | `putIfAbsent` the owner (claim) | | lost the `putIfAbsent` race | re-read; equal uuid proceeds, else `CORRUPTED_DATA` | "Provably empty" requires both an authoritative decoded catalog naming no life owned by `server_root_id` and @@ -261,7 +266,8 @@ which any reclaim is instant. **Permanent removal** of a dead replica (`Cas::decommissionPoolMember`, driven by `SYSTEM CAS DROP POOL MEMBER '' FROM DISK ''`) claims the victim's mount slot -as an administrative writer with a no-wait policy (refuses immediately if the member is alive), +as an administrative writer with a no-wait policy (refuses unless the slot is terminal: +`terminated` or `fenced`; an `expired` slot is refused until the disk's GC leader fences it), drops every ref-bearing namespace, sweeps manifest debris before the slot (deleting the mount removes the watermark authority), drains staging and roots, then — only with zero warnings — retires in order: mount, epoch, a final liveness re-check, owner tombstone. diff --git a/docs/en/antalya/cas/architecture/part-lifecycle.md b/docs/en/antalya/cas/architecture/part-lifecycle.md index 8d2e7175e5518..62d4f8ca45911 100644 --- a/docs/en/antalya/cas/architecture/part-lifecycle.md +++ b/docs/en/antalya/cas/architecture/part-lifecycle.md @@ -109,7 +109,7 @@ something recoverable; no row loses data or leaves a dangling reference. | # | Crash window | Left behind | Who cleans it | |---|---|---|---| -| C1 | During staging | Local temp files, or S3 staging objects | Local: unconditional cleanup plus buffer destructor. S3: the mount's own staging sweep at next mount — never deleted on abort | +| C1 | During staging | Local temp files, or S3 staging objects | Local: unconditional cleanup plus buffer destructor. S3: the transaction deletes it on commit or abort (best-effort); after a crash, the next mount of the same `server_root_id` sweeps it, or `SYSTEM CAS DROP POOL MEMBER` drains it | | C2 | After `stageManifest`, before `precommitAdd` | An unreferenced manifest body | Writer's best-effort exact-token delete; durable backstop is the orphan-manifest sweep | | C3 | `precommitAdd` returned Unresolved | A possibly-live precommit binding | Intent recorded pre-append; `abandon` appends the exact removal, tolerating absence. The body is never writer-deleted | | C4 | Between `precommitAdd` and `promote` | A live precommit plus uploaded blobs | No resume path exists. Removed by `abandon`, else by a fenced successor's stale-precommit sweep | diff --git a/docs/en/antalya/cas/bucket-requirements.md b/docs/en/antalya/cas/bucket-requirements.md index ea4bfe0dbafed..a42a6714998a4 100644 --- a/docs/en/antalya/cas/bucket-requirements.md +++ b/docs/en/antalya/cas/bucket-requirements.md @@ -45,6 +45,12 @@ Because that check is part of the mount battery, `skip_access_check = true` is r generation-token disk. Mount the disk read-only if you need to start before the access check can pass. +**Incomplete multipart uploads.** With `cas_staging_backend = s3`, a process killed during a +multipart upload leaves an incomplete upload under `staging//`. Object `LIST` does +not show it, so neither the transaction's own delete nor the mount-start sweep can remove it, and its +parts stay billed. Add an `AbortIncompleteMultipartUpload` lifecycle rule on the bucket (one day is +enough: a staging upload completes within its own transaction or never). + ## Soft delete is an operator precondition {#soft-delete} Object **soft delete must be disabled** on a `CAS` bucket, and unlike versioning this one is *not* diff --git a/docs/en/antalya/cas/configuration.md b/docs/en/antalya/cas/configuration.md index 6ec654bc0b78e..07e134a9deb55 100644 --- a/docs/en/antalya/cas/configuration.md +++ b/docs/en/antalya/cas/configuration.md @@ -89,7 +89,7 @@ entirely before release. Treat this table as a snapshot of the current build, no | Setting | Default | Description | |---|---|---| -| `cas_server_root_id` | — (required) | Explicit layout subtree identity; macros expand as in the `s3` `endpoint`. Anchored in the pool by a write-once owner claim — a colliding identity is refused at mount | +| `cas_server_root_id` | — (required) | Explicit layout subtree identity; macros expand as in the `s3` `endpoint`. Anchored in the pool by a write-once owner claim — a colliding identity is refused at mount. May not contain `/`, also after macro expansion: `SYSTEM CAS DROP POOL MEMBER` selects objects by key prefix, so a member `a/b` would be drained by a decommission of `a`. A disk configured with such an id refuses to open, including one whose id already owns its owner object in the pool; see [recreating a member whose id contains `/`](/antalya/cas/operations/migration#recreate-id-with-slash) | | `cas_scratch_path` | `/disks//cas_scratch/` | Server-local scratch dir for the write-buffer spill; a relative value is anchored to the server data path | | `cas_gc_enabled` | `true` | Run the background GC scheduler on this disk. `false` is a debugging aid, not an operating mode: garbage then accumulates indefinitely and silently — watch `system.cas_gc_log` for round activity if you ever toggle it | | `cas_gc_interval_sec` | `60` | Seconds between background GC rounds (≥ 1) | @@ -110,7 +110,7 @@ entirely before release. Treat this table as a snapshot of the current build, no | `cas_attempt_timeout_ms` | `5000` | Budget for one HTTP attempt of a writable Native mount's control-plane requests (read, head, list, remove, conditional write), at least 1. Together with the connect cap it forms the attempt envelope (`cas_attempt_timeout_ms + 2 × cap`; the cap is `cas_attempt_timeout_ms` itself when the disk's `connect_timeout_ms` is `0`, else `min(connect_timeout_ms, cas_attempt_timeout_ms)`) that the lease arithmetic reserves: one TCP connect and one TLS handshake under the cap each, send/receive bounded per socket operation by `cas_attempt_timeout_ms`. With background renewal the cadence check requires `cas_mount_renew_period_ms + 2 × envelope + cas_lease_safety_margin_ms < cas_mount_lease_ttl_ms`, which puts an effective ceiling on the frozen connect cap: under the defaults (TTL 30000, period 10000, margin 2000) the envelope must stay under 9000, so a disk `connect_timeout_ms` of 2000 ms or more refuses to open writable — lower the connect timeout or raise the TTL if you hit this. Blob body publication does not use this budget: its object-creating request is sent once under the disk's `request_timeout_ms`, with the connect timeout under the same cap | | `cas_lease_safety_margin_ms` | `2000` | Startup-only margin validated against the mount lease TTL: the attempt envelope + `cas_lease_safety_margin_ms` must be strictly less than the mount lease TTL, and `cas_mount_renew_period_ms` + 2 × envelope + `cas_lease_safety_margin_ms` too, or the disk refuses to open writable | | `cas_unsafe_remount_no_delay` | `0` | Reclaim a mount slot that carries this server's own uuid at once after a hard restart, without observing the slot's token for the lease TTL. Unsafe whenever two processes can hold the same `server_uuid` (a copied uuid file, a stalled predecessor). After such a reclaim the predecessor can still start conditional writes until its own cutoff (`confirmed deadline − cas_lease_safety_margin_ms − 2 × envelope`) or until its next renewal meets the token guard, and a request it already sent may still materialize later. That is not a data hazard: ref-log keys carry `(writer_epoch, sequence)` and creates are conditional, so two writers can never commit different bodies to one key, and recovery's epoch seal settles any straggler (recovery fails closed after 64 successive seal-create attempts displaced by newly materializing old-epoch transactions). The exposure is availability, not data. Intended for test stands and deployments that guarantee one process per uuid | -| `cas_staging_backend` | `local` | Blob staging backend (`local` \| `s3`); `s3` is opt-in and requires native same-store copy on writable mount | +| `cas_staging_backend` | `local` | Blob staging backend (`local` \| `s3`); `s3` is opt-in and requires native same-store copy on writable mount. A transaction deletes its staging objects on commit and on abort; mount start removes leftovers of the same `server_root_id`. Neither can see a multipart upload that was never completed, so add an `AbortIncompleteMultipartUpload` lifecycle rule on the bucket to reclaim those | Both settings are local. Startup reclaim and GC's fence-out judge liveness by the mount slot's write token holding stable on the observer's own `CLOCK_BOOTTIME` for `L + floor(L / 20) + step`. `L` is the diff --git a/docs/en/antalya/cas/operations/migration.md b/docs/en/antalya/cas/operations/migration.md index e8258b96d61d3..88927d4cfc996 100644 --- a/docs/en/antalya/cas/operations/migration.md +++ b/docs/en/antalya/cas/operations/migration.md @@ -202,12 +202,30 @@ mount slot stays in place as a resume anchor rather than being fully retired. **Preconditions.** Confirm the member is actually and permanently dead before running this: the operation fences that `server_root_id` out even if the server comes back online, and it deletes -namespace and drain state that cannot be recovered. Check `system.cas_mounts` for the member's -`state` and `last_success_age_seconds` first — a `live` row, or one with a recent lease renewal, -means the member is not a decommission candidate yet. +namespace and drain state that cannot be recovered. The command claims the slot without waiting +and trusts no wall clock, so it accepts only a terminal slot in `system.cas_mounts`: +`state = 'terminated'` (clean stop) or `state = 'fenced'` (GC fenced the lease). A `live` or `expired` +row is refused with `alive or contended`; a hard-killed member stays `expired` until the disk's GC leader fences it at the start of a GC round, once it has seen the lease unchanged for about one +`cas_mount_lease_ttl_ms` plus one `cas_mount_renew_period_ms`; the wait can be one GC round longer. `SYSTEM CAS GC RUN` on the leader +(`is_leader = 1` in `system.cas_mounts`) can trigger the fence, but the run that fences must come at +least that long after the first run that saw the lease; two back-to-back runs do not fence. **Verification.** After the command reports `slot_removed = true` with no warnings, the member's `server_root_id` no longer appears as a row in `system.cas_mounts` on any peer, and a subsequent `SYSTEM CAS GC RUN` on the pool will no longer wait on or fence its heartbeat. See [mount, unmount, crash](/antalya/cas/architecture/mounts-and-leases#mount-lifecycle) for how the claim, drain, and retirement steps fit into the mount-slot lifecycle. + +## Recreating a member whose id contains `/` {#recreate-id-with-slash} + +A `cas_server_root_id` that contains `/` is refused: the disk fails to open with `BAD_ARGUMENTS` and a +message that contains `contains '/'`. Pools created by earlier builds can hold such an id. To bring the +server back: + +1. Set `` to a new value without `/`, for example `shard-01-replica-a` instead of + `shard-01/replica-a`. If the id comes from macros, remove the `/` from the template. +2. Restart the server. The disk claims the new id over an empty subtree. + +Data stored under the old id is not visible under the new id. The old id's objects stay in the pool +and keep their blobs referenced. `SYSTEM CAS DROP POOL MEMBER` and `clickhouse-disks cas-drop-member` +refuse an id with `/`, so they cannot reclaim them. diff --git a/docs/en/antalya/cas/operations/troubleshooting.md b/docs/en/antalya/cas/operations/troubleshooting.md index ef63acf17a337..a3d536d8518bc 100644 --- a/docs/en/antalya/cas/operations/troubleshooting.md +++ b/docs/en/antalya/cas/operations/troubleshooting.md @@ -22,6 +22,10 @@ tools. | A dangling-access exception or `CORRUPTED_DATA` on read | Run `clickhouse-disks cas-fsck --detail` and check `dangling` specifically — it is the one class that means data loss, distinct from `unreachable`/`awaiting-gc`, which are just waiting for graduation | A nonzero `dangling` count is a real incident: collect the `--detail` output (see [what to collect before filing a bug](/antalya/cas/operations/debugging#filing-a-bug)) before taking any destructive action | | `SYSTEM CAS FSCK` or `clickhouse-disks cas-fsck` times out on a large pool | The scan is bounded by `--timeout` (default 600s / the `SYSTEM` form has no override); a large `roots/` prefix can make the scan slow | Retry with `--partial` to see the counts accumulated so far instead of aborting empty-handed, or `--namespace ` to scope the scan to a subset of namespaces | | `SYSTEM CAS DROP POOL MEMBER` returns a non-empty `warnings` column | A per-object drain step could not confirm emptiness; the mount slot is left terminated but not fully drained, as a resume anchor | Rerun the same command — it is resumable and skips namespaces already marked removed, reporting them under `namespaces_already_removed` | +| `SYSTEM CAS DROP POOL MEMBER` refuses with `alive or contended` although the member is down | The claim trusts no wall clock: `expired` in `system.cas_mounts` is not a certificate of death; only a terminal slot (`terminated` or `fenced`) is reclaimable | Wait until the member's row shows `state = 'terminated'` (clean stop) or `state = 'fenced'` (the disk's GC leader fences the stale lease at the start of a GC round, once it has seen it unchanged for about one `cas_mount_lease_ttl_ms` plus one `cas_mount_renew_period_ms`, so the wait can be one GC round longer; `SYSTEM CAS GC RUN` on the leader, `is_leader = 1` in `system.cas_mounts`, can trigger the fence, but the run that fences must come at least that long after the first run that saw the lease), then rerun | +| A server refuses to start with `owned by a different server` | The owner object binds the `server_root_id` to another `server_uuid`: the local uuid file was regenerated, or two servers share one `cas_server_root_id` | Restore the uuid file (keep it on a persistent volume); or start this server under a new `cas_server_root_id`. To reclaim the old id's data once its owning server is permanently gone, decommission it with `SYSTEM CAS DROP POOL MEMBER` from a surviving member, once its slot is `terminated` or `fenced`; the two steps are independent, and in a single-member pool starting under the new id comes first, because a hard-killed old id is fenced only by a running GC leader | +| A `CAS` disk refuses to open with `contains '/'` | Its `cas_server_root_id` contains `/`, directly or through a macro; such ids are refused, including ids that already own their owner object in the pool | Configure a new `cas_server_root_id` without `/` and restart; see [recreating a member whose id contains `/`](/antalya/cas/operations/migration#recreate-id-with-slash) | +| A server refuses to start with `explicitly decommissioned` | Its `server_root_id` was retired by `SYSTEM CAS DROP POOL MEMBER`; the tombstone is permanent so writer epochs never repeat | Configure a new `cas_server_root_id` for the disk; do not clear the tombstone by hand | | Writes or `ALTER`s on a `CAS` disk fail with a `READONLY`-class error | The disk's metadata storage rejects every mutating entry point; this is deliberate for a disk opened with `true`, used by every offline `clickhouse-disks` tool | Confirm whether the disk was intentionally configured read-only (offline inspection, `cas-fsck`, `cas-gc-dryrun`, `cas-gc-rebuild`, `cas-drop-member` all require it); a production disk serving writes must not carry `true` | | A table stays unavailable after a transient network error during startup | `AsyncLoader` has no retry/requeue path for a failed table load job: a transient S3 `NETWORK_ERROR` during `CAS` ref-table startup recovery can leave the job permanently `FAILED` | Restart the server, or issue a fresh load for the table; this is a one-shot job design, not a `CAS`-specific bug | | A mounted pool directory was removed or renamed out of band | Renewal observes an absent, foreign, successor, or otherwise conflicting mount body and terminates the keeper with a typed fail-closed exception; the runtime closes the local write fence and requests remount rather than adopting the body | Never remove or rename a live pool's storage path. To retire a member permanently use [`SYSTEM CAS DROP POOL MEMBER`](/antalya/cas/operations/migration#decommission) instead of raw filesystem operations; collect the `watermark_renew` classification (`vanished` for an absent body, `conflict` for a foreign, successor, or otherwise conflicting one) and the subsequent `mount_remount` step | diff --git a/docs/en/sql-reference/statements/system.md b/docs/en/sql-reference/statements/system.md index a6e3233fa5ae9..f82f6310f9d0e 100644 --- a/docs/en/sql-reference/statements/system.md +++ b/docs/en/sql-reference/statements/system.md @@ -594,6 +594,13 @@ are confirmed. This is a **destructive, irreversible** operation: only run it on confirmed permanently dead, since it fences the member out even if it later comes back online, and it deletes namespace and drain state that cannot be recovered. +The claim accepts only a terminal mount slot (`state` `terminated` or `fenced` in +`system.cas_mounts`). A hard-killed member shows `expired` until the disk's GC leader fences it at the start of a GC round, once it has +seen the lease unchanged for about one lease TTL plus one renew period; the wait can be one GC round longer. Until then the command +refuses with `alive or contended`. `SYSTEM CAS GC RUN` on the leader (`is_leader = 1` in +`system.cas_mounts`) can trigger the fence, but the run that fences must come at least that long after +the first run that saw the lease; two back-to-back runs do not fence. + It is a writer operation, not GC: it emits ordinary ref-edge deltas rather than inventing GC transitions, and it does not synchronously reclaim shared blob content — removing the ref edges only makes the now-unreferenced blobs eligible for an ordinary GC round to reclaim later. @@ -602,8 +609,9 @@ makes the now-unreferenced blobs eligible for an ordinary GC round to reclaim la SYSTEM CAS DROP POOL MEMBER 'server_root_id' FROM DISK 'disk_name' [ON CLUSTER cluster_name] ``` -Both `server_root_id` and `disk_name` are required string literals (a `server_root_id` is an opaque server-root path -that may contain `/`, not a plain identifier, so it cannot be written unquoted). +Both `server_root_id` and `disk_name` are required string literals (a `server_root_id` is an opaque string, not a +plain identifier, so it cannot be written unquoted). A `server_root_id` that contains `/` is refused with +`BAD_ARGUMENTS`. The operation is resumable: a rerun skips namespaces already marked `Removed` and reports them separately from namespaces newly removed by this invocation. Per-object drain failures are recorded diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.cpp b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.cpp index 3a78e4a5eb3f7..fb1c7fc926461 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.cpp +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.cpp @@ -108,9 +108,8 @@ ContentAddressedTransaction::ContentAddressedTransaction(ContentAddressedMetadat ContentAddressedTransaction::~ContentAddressedTransaction() { - /// Always clean up pending staging (whether committed or not). On the success path - /// cleanupPendingTempFiles was already called at the end of commit(); this call is the defensive - /// backstop for aborted/exception-unwound transactions whose publishStaging never ran. + /// On abort this is the cleanup of local and S3 staging. After a commit it finds nothing left, + /// except when the commit published some parts and then threw. cleanupPendingTempFiles(); /// An uncommitted transaction's uploads become min_active_build_sequence-spared debris: abandon every @@ -175,37 +174,20 @@ void ContentAddressedTransaction::cleanupPendingTempFiles() noexcept std::error_code ec; std::filesystem::remove(pb.staging_key, ec); } - else if (committed) + else { - /// A successful commit deletes the S3 staging object - /// HERE — `committed` is only ever true when EVERY part's `publishStaging` ran to - /// completion (commit() sets it right before this call), which means every referenced - /// pending blob was already materialized (`PartWriteTxn::putBlob` → `publishBlob`) - /// or, for an orphaned pending blob (its entry removed by `unlinkFile`/`replaceFile` - /// before commit), was never going to be promoted at all — either way the staging object - /// is no longer needed as a re-readable publication source, so it is safe to reclaim now. - /// - /// An ABORTED/exception-unwound transaction (`committed == false`, including a partial - /// multi-part commit failure where an EARLIER part's blobs were already promoted) leaves - /// its S3 staging objects in place — `staging_key` is a remote object-storage key, never a - /// bare `fs::remove` target, and is reclaimed by the mount-lease-scoped sweeper - /// (`Cas::sweepOwnMountStaging`), never here. This mirrors the local path's own asymmetry: - /// `Local` staging is a private per-transaction scratch file removed unconditionally on - /// both commit and abort (nobody else can ever read it), whereas an `S3` staging object - /// is the sanctioned re-readable source for publication and must outlive an aborted - /// transaction so a later attempt (or the sweeper) can still account for it. + /// The S3 staging object is private to this transaction and nothing reads + /// `pending_blobs` after this call, so commit and abort delete it alike. An object that + /// outlived an abort would stay until the next mount of this `server_root_id`. try { metadata_storage.objectStorage()->removeObjectIfExists(StoredObject(pb.staging_key)); } catch (...) // NOLINT(bugprone-empty-catch) { - /// Best-effort (noexcept context): a stubborn delete just leaves debris for the - /// mount-lease sweeper to reclaim on a later mount. + /// Best-effort (noexcept context): a stubborn delete leaves debris for the mount-start sweep. } } - /// else: an S3-mode pending blob of an ABORTED transaction — intentionally left in place - /// (see above); the mount-lease sweeper (`Cas::sweepOwnMountStaging`) is its reclaimer. } st.pending_blobs.clear(); } diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.h b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.h index 60e473ede5831..44899e93a0931 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.h +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.h @@ -33,8 +33,8 @@ namespace DB /// /// Content blobs are first represented by local or S3 staging objects and are uploaded only during /// publication. Inline entries remain in the manifest tree when they fit `INLINE_CAP`. The -/// destructor removes private local staging, leaves aborted S3 staging for the mount-lease sweeper, -/// and abandons any open part builds; it never publishes an uncommitted ref. +/// destructor removes the transaction's local and S3 staging and abandons any open part builds; +/// it never publishes an uncommitted ref. class ContentAddressedTransaction : public IMetadataTransaction { public: @@ -145,9 +145,8 @@ class ContentAddressedTransaction : public IMetadataTransaction /// loop is idempotent (never re-publishes a staging). /// `staging_key` is either a private local temp path or an S3 staging object key returned by - /// `CaContentWriteBuffer` at finalize. The backend selects cleanup and read-your-writes: - /// local files are removed by this transaction, whereas aborted S3 objects must remain - /// available to the mount-lease sweeper and to recovery of the promote source. + /// `CaContentWriteBuffer` at finalize. The backend selects the read-your-writes source; both + /// kinds are removed by this transaction on commit and on abort. struct PendingBlob { Cas::BlobRef ref; std::string staging_key; uint64_t size = 0; Cas::StagingBackend backend = Cas::StagingBackend::Local; }; std::vector pending_blobs; /// Staged blobs uploaded after the manifest edge is precommitted. }; @@ -192,8 +191,8 @@ class ContentAddressedTransaction : public IMetadataTransaction /// Parses a disk path and maps a part-file path to its namespace/ref/file route. std::optional routeOf(const std::string & path) const; - /// Removes local staging files after commit or abort. On a successful commit it also removes - /// S3 staging objects; aborted S3 objects are intentionally retained for lease-scoped cleanup. + /// Removes the transaction's staging after commit or abort: local files and S3 staging objects + /// alike. S3 deletes are best-effort; a lost delete leaves debris for the mount-start sweep. void cleanupPendingTempFiles() noexcept; /// Uploads through `st.build` only the pending blobs still referenced by `st.entries`. An entry diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.cpp b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.cpp index 83d21a39500d7..60d96bcf5964e 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.cpp +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasPool.cpp @@ -754,9 +754,9 @@ void Pool::mountWritable(PoolPtr & store, UInt128 our_uuid, MountClaimPolicy pol if (policy == MountClaimPolicy::NoWait) { /// No FORCE variant, no wait-and-observe: the decommission gate treats any live-looking - /// or foreign-owner lease as an immediate refusal. A raced write that observed nobody - /// has no holder to describe, and the lease this server merely proposed is not one -- - /// naming it would send an operator after this very process. + /// or foreign-owner lease as an immediate refusal, and names the terminal states it + /// accepts, because `expired` in system.cas_mounts is wall-clock arithmetic this claim + /// never trusts. A raced write that observed nobody has no holder to describe. const String holder = claim.body ? fmt::format("mount lease held by uuid={} epoch={} pid={} hostname={} (expires_at_ms={})", u128ToHex(claim.body->server_uuid), claim.body->writer_epoch, @@ -764,9 +764,17 @@ void Pool::mountWritable(PoolPtr & store, UInt128 our_uuid, MountClaimPolicy pol : String("the conditional write that lost the mount slot observed nothing at the key, " "so the holder is unknown to this server"); throw Exception(ErrorCodes::ABORTED, - "CAS decommission '{}': pool member is alive or contended — {}. Refusing (no FORCE " - "variant exists; stop the server or wait for its lease to lapse).", - srid, holder); + "CAS decommission '{}': pool member is alive or contended — {}. Refusing; there is no FORCE " + "variant. A member can be removed only once its mount slot is terminal in system.cas_mounts " + "(server_root_id = '{}'): state = 'terminated' (the server stopped cleanly) or state = 'fenced' " + "(GC fenced its lease). A hard-killed member shows state = 'expired' first and is not removable " + "yet. The disk's GC leader fences it at the start of a GC round, once it has seen the lease unchanged " + "for about one cas_mount_lease_ttl_ms plus one cas_mount_renew_period_ms, so the wait can be one " + "GC round longer. SYSTEM CAS GC RUN on the leader (is_leader = 1 in system.cas_mounts) can " + "trigger the fence, but the run that fences must come at least that long after the first run " + "that saw the lease. " + "Then rerun this command.", + srid, holder, srid); } /// LiveDoubleStart (waited out the bound → a live twin) or ForeignOwner → fail closed /// with the actionable, multi-line startup error. diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.cpp b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.cpp index 4cde128842332..fc070b7bee29e 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.cpp +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.cpp @@ -445,10 +445,10 @@ void throwIfOwnerRetired(const OwnerObject & owner, const String & srid) return; throw Exception(ErrorCodes::CORRUPTED_DATA, - "CAS server-root '{}' was explicitly decommissioned by an operator (tombstoned at {} ms) " - "and is refusing to silently resume — if you genuinely intend to bring this server-root " - "back, manually clear the owner object's tombstone field and restart " - "(same manual-recovery pattern as an owner anchor lost over existing data)", + "CAS server-root '{}' was explicitly decommissioned (owner tombstoned at {} ms by " + "SYSTEM CAS DROP POOL MEMBER) and refuses to start again: writer epochs of a retired id must never " + "repeat. To run a server on this pool again, configure a new for this disk and " + "keep the local uuid file. Do not clear the tombstone by hand.", srid, *owner.retired_at_ms); } } @@ -540,17 +540,21 @@ void claimOwnerOrThrow( throwIfOwnerRetired(*owner, srid); return; } - /// Mirror mountDoubleStartMessage's operator guidance: the by-far most common cause is a - /// REGENERATED local ClickHouse uuid file (wiped /var/lib/clickhouse, a pod rescheduled - /// without a persistent volume) while the pool kept the old identity — name it and the - /// recovery options instead of a bare refusal. + /// The common cause is a regenerated local uuid file (wiped /var/lib/clickhouse, a pod recreated + /// without a persistent volume) while the pool kept the old identity; name it and the supported + /// ways out, with the hand delete last: it is refused anyway while the subtree holds data. throw Exception(ErrorCodes::CORRUPTED_DATA, "CAS server-root '{}' is owned by a different server (owner server_uuid={}, ours={}) — refusing to claim. " - "This usually means THIS server's local uuid file was regenerated (e.g. /var/lib/clickhouse was wiped, " - "or the container/pod was recreated without a persistent volume) while the pool kept the old identity. " - "Recover by restoring the old local uuid file; or configure a fresh for this disk; " - "or — only after verifying that NO server uses this root — manually delete the owner object '{}' and restart.", - srid, u128ToHex(owner->server_uuid), u128ToHex(our_uuid), key); + "Either this server's local uuid file was regenerated (/var/lib/clickhouse wiped, or a pod recreated " + "without a persistent volume) while the pool kept the old identity, or two servers are configured with " + "the same . Recover by restoring the old uuid file, or by starting this " + "server under a new . To reclaim the old id's data once its owning server is " + "permanently gone, decommission it — SYSTEM CAS DROP POOL MEMBER '{}' FROM DISK '' on a " + "surviving member (offline: clickhouse-disks cas-drop-member) — once its slot in system.cas_mounts is " + "'terminated' or 'fenced'. " + "Deleting the owner object '{}' by hand is a last resort, only after verifying that NO server uses this " + "root; it is refused while the subtree still holds data.", + srid, u128ToHex(owner->server_uuid), u128ToHex(our_uuid), srid, key); } /// Owner absent. Claiming is allowed ONLY over a provably-empty subtree; an absent owner over diff --git a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.h b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.h index b9e40be971a28..896da9db167e6 100644 --- a/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.h +++ b/src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Pool/CasServerRoot.h @@ -76,14 +76,10 @@ struct MountRenewOperationEnvironment }; /// Validate a `server_root_id` — the explicit, configured identity of the content-addressed layout -/// subtree a server owns. It is a clean relative path: it composes into the -/// object-key tree (`gc/server-roots//...`, `roots//...`), so the same hygiene the layout -/// applies to a namespace applies here (mirrors `CasLayout.h::checkNamespace`): -/// - non-empty; -/// - no leading/trailing '/', no empty segment ("//"); -/// - no '.' or '..' segment; -/// - total length <= 255; -/// - no segment equal to the reserved "_files" / "_manifests". +/// subtree a server owns. It composes into object keys (`gc/server-roots//...`, `roots//...`) +/// as ONE path component: non-empty, at most 255 bytes, no '/', not '.' or '..', not the reserved +/// "_files" / "_manifests". A '/' is refused even for an id that already owns its owner object, because +/// decommission and the staging sweep select a member's objects by key prefix. /// Throws `ErrorCodes::BAD_ARGUMENTS` on any violation. Fail closed — there is no sanitizing fallback. inline void validateServerRootId(const String & id) { @@ -94,27 +90,22 @@ inline void validateServerRootId(const String & id) throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "server_root_id '{}' is too long ({} > 255 bytes)", id, id.size()); - size_t start = 0; - while (true) - { - size_t end = id.find('/', start); - const String segment = id.substr(start, end == String::npos ? String::npos : end - start); - if (segment.empty()) - throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, - "server_root_id '{}' has an empty segment (leading/trailing or doubled '/')", id); - if (segment == "." || segment == "..") - throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, - "server_root_id '{}' uses a relative segment ('.' or '..')", id); - if (segment == "_files") - throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, - "server_root_id '{}' uses the reserved segment '_files'", id); - if (segment == "_manifests") - throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, - "server_root_id '{}' uses the reserved segment '_manifests'", id); - if (end == String::npos) - break; - start = end + 1; - } + if (id.find('/') != String::npos) + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, + "server_root_id '{}' contains '/'. A decommission selects a member's objects by key prefix, so a " + "member 'a/b' would be drained by a decommission of 'a'. Recreate the server that uses this id under a new " + " without '/'; the objects of the old id stay in the pool and are not visible " + "under the new id", + id); + if (id == "." || id == "..") + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, + "server_root_id '{}' uses a relative segment ('.' or '..')", id); + if (id == "_files") + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, + "server_root_id '{}' uses the reserved segment '_files'", id); + if (id == "_manifests") + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, + "server_root_id '{}' uses the reserved segment '_manifests'", id); } /// The per-server-root control objects (owner, epoch, and mount lease) and their text codecs are kept @@ -148,9 +139,9 @@ std::optional readOwnerUuid(CasOperation & op, const Layout & l, const /// - owner present, equal `our_uuid`, and not tombstoned → ok (return); /// - owner present and tombstoned → throw `CORRUPTED_DATA` (explicitly retired — fail closed); /// - owner present, different → throw `CORRUPTED_DATA` (foreign owner — fail closed); -/// - owner absent AND the subtree is provably empty → `create` the owner (claim); /// - owner absent BUT the subtree is non-empty → throw `CORRUPTED_DATA` (identity lost over -/// existing data — never silently re-claim). +/// existing data — never silently re-claim); +/// - owner absent, subtree provably empty → `create` the owner (claim). /// The owner object is never deleted and never reassigned to a different UUID. void claimOwnerOrThrow( CasOperation & op, const Layout & l, const String & srid, UInt128 our_uuid, @@ -612,11 +603,10 @@ class MountLeaseRenewer /// Mount-lease-scoped staging sweeper for objects left behind by S3-native staging. /// -/// A leaked S3 staging object happens two ways: (1) an exception between blob publication succeeding and -/// `cleanupPendingTempFiles` deleting the staging key (`ContentAddressedTransaction.cpp`), or (2) an -/// aborted/cancelled transaction whose pending blobs were staged but never promoted — by design, -/// `cleanupPendingTempFiles` deliberately leaves an S3 staging object in place on the abort path (never a -/// bare `fs::remove` on a remote key), so this sweeper is its ONLY reclaimer. Debris from either case is +/// A leaked S3 staging object is one whose transaction never ran `cleanupPendingTempFiles` (the process +/// died, or an exception hit between `sink->finalize()` and `on_finalized` registering the key in +/// `CaContentWriteBuffer::finalizeImpl`) or whose best-effort delete on commit or abort was lost. While +/// the member lives, this sweeper is its only reclaimer. Debris from either case is /// bounded to `staging//` — the ONE mount that could ever have written under that prefix, since /// every staging key this mount ever mints comes from `ContentAddressedMetadataStorage::stagingKeyPrefix()` /// (`physicalKey(pool_prefix + "/staging/" + server_root_id)`), keyed by THIS mount's own `server_root_id`. diff --git a/src/Disks/tests/gtest_cas_decommission.cpp b/src/Disks/tests/gtest_cas_decommission.cpp index 936e9655e9b14..e4ac5fe8c9916 100644 --- a/src/Disks/tests/gtest_cas_decommission.cpp +++ b/src/Disks/tests/gtest_cas_decommission.cpp @@ -505,6 +505,29 @@ TEST(CASDecommission, RefusesLiveMember) }); } +/// The refusal names the state a hard-killed member has to reach. `expired` in `system.cas_mounts` is +/// wall-clock arithmetic; the claim trusts only a terminal slot (`terminated` or `fenced`), and an +/// expired, unfenced lease takes exactly the branch an unexpired one takes here. +TEST(CASDecommission, NoWaitRefusalNamesTheTerminalStates) +{ + auto backend = std::make_shared(); + auto victim = openVictim(backend); + + try + { + Pool::openForDecommission(backend, PoolConfig{.pool_prefix = "p", .server_root_id = "admin"}, "victim"); + FAIL() << "expected the live member to be refused"; + } + catch (const DB::Exception & e) + { + EXPECT_EQ(e.code(), ErrorCodes::ABORTED); + EXPECT_NE(e.message().find("alive or contended"), String::npos) << e.message(); + EXPECT_NE(e.message().find("state = 'terminated'"), String::npos) << e.message(); + EXPECT_NE(e.message().find("state = 'fenced'"), String::npos) << e.message(); + EXPECT_NE(e.message().find("GC round"), String::npos) << e.message(); + } +} + TEST(CASDecommission, ClaimsDeadMemberAndBumpsEpoch) { auto backend = std::make_shared(); @@ -547,6 +570,31 @@ TEST(CASDecommission, RefusesUnknownMember) }); } +/// `SYSTEM CAS DROP POOL MEMBER 'a/b'` is refused before it touches the member, even when `a/b` owns its +/// owner object: the drain selects objects by key prefix, so it is defined only for ids without `/`. +TEST(CASDecommission, RefusesIdWithSlash) +{ + auto backend = std::make_shared(); + const Layout layout("p"); + DB::Cas::tests::seedPoolMetaForRestart(*backend); + OperationForTest raw_op(*backend); + ASSERT_TRUE(std::holds_alternative((*raw_op).create(layout.ownerKey("shard-01/replica-a"), + encodeOwner(OwnerObject{.server_uuid = UInt128(1), .retired_at_ms = std::nullopt}), Retry::once()))); + const auto owner_before = (*raw_op).read(layout.ownerKey("shard-01/replica-a"), Retry::once()); + ASSERT_TRUE(owner_before.has_value()); + + DB::Cas::tests::expectThrowsCodeWithMessage(ErrorCodes::BAD_ARGUMENTS, "contains '/'", [&] + { + decommissionPoolMember(backend, PoolConfig{.pool_prefix = "p", .server_root_id = "admin"}, "shard-01/replica-a"); + }); + + const auto owner_after = (*raw_op).read(layout.ownerKey("shard-01/replica-a"), Retry::once()); + ASSERT_TRUE(owner_after.has_value()); + EXPECT_EQ(owner_after->etag, owner_before->etag); + EXPECT_FALSE((*raw_op).head(layout.mountKey("shard-01/replica-a"), Retry::once()).has_value()); + EXPECT_FALSE((*raw_op).head(layout.epochKey("shard-01/replica-a"), Retry::once()).has_value()); +} + TEST(CASDecommission, SecondConcurrentDecommissionRefused) { auto backend = std::make_shared(); diff --git a/src/Disks/tests/gtest_cas_mount.cpp b/src/Disks/tests/gtest_cas_mount.cpp index d36d084c3ed63..41a6baf861c62 100644 --- a/src/Disks/tests/gtest_cas_mount.cpp +++ b/src/Disks/tests/gtest_cas_mount.cpp @@ -121,10 +121,10 @@ class OwnerConflictRevealsManifestBackend : public InMemoryBackend const String & key, const String & bytes, const std::optional & expected_value, TransportAccess & access) override { - if (!fired && !expected_value && key == "p/gc/server-roots/root/x/owner") + if (!fired && !expected_value && key == "p/gc/server-roots/rootx/owner") { fired = true; - InMemoryBackend::write("p/cas/manifests/root/x/table/debris", "x", std::nullopt, access); + InMemoryBackend::write("p/cas/manifests/rootx/table/debris", "x", std::nullopt, access); return std::unexpected(RawConflict{}); } return InMemoryBackend::write(key, bytes, expected_value, access); @@ -490,7 +490,7 @@ TEST(CASMountAudit, RenewalDefaultLogsAreBounded) TEST(CASServerRootId, ValidationAcceptsCleanPathsRejectsBad) { EXPECT_NO_THROW(validateServerRootId("replica-a")); - EXPECT_NO_THROW(validateServerRootId("shard-01/replica-a")); + EXPECT_THROW(validateServerRootId("shard-01/replica-a"), DB::Exception); EXPECT_THROW(validateServerRootId(""), DB::Exception); EXPECT_THROW(validateServerRootId("/replica"), DB::Exception); EXPECT_THROW(validateServerRootId("replica/"), DB::Exception); @@ -499,6 +499,30 @@ TEST(CASServerRootId, ValidationAcceptsCleanPathsRejectsBad) EXPECT_THROW(validateServerRootId("a/_files/b"), DB::Exception); } +/// Decommission and the staging sweep select a member's objects by key prefix, so a member `a/b` would be +/// drained by a decommission of `a`. An id with `/` is refused even when its owner object already exists. +TEST(CASServerRootId, ExistingIdWithSlashRefusesToOpen) +{ + auto b = std::make_shared(); + const Layout l("p"); + DB::Cas::tests::seedPoolMetaForRestart(*b); + { + Ops seed(b); + mustCommit(seed.op.create(l.ownerKey("shard-01/replica-a"), + encodeOwner(OwnerObject{.server_uuid = UInt128(1), .retired_at_ms = std::nullopt}), + Retry::standard()), "owner of an existing nested member"); + } + DB::Cas::tests::expectThrowsCodeWithMessage(DB::ErrorCodes::BAD_ARGUMENTS, "new without '/'", [&] + { + Pool::open(b, PoolConfig{.pool_prefix = "p", .server_id = UInt128(1), .server_root_id = "shard-01/replica-a"}); + }); + Ops check(b); + EXPECT_FALSE(check.op.head(l.mountKey("shard-01/replica-a"), Retry::standard()).has_value()) + << "a refused open must not claim the mount slot"; + EXPECT_FALSE(check.op.head(l.epochKey("shard-01/replica-a"), Retry::standard()).has_value()) + << "a refused open must not mint an epoch"; +} + TEST(CASServerRoot, KeysAndCodecsRoundTrip) { Layout layout("p"); @@ -594,6 +618,37 @@ TEST(CASServerRootClaim, TombstonedSameOwnerFailsClosed) } } +/// Each identity refusal names the supported recovery: the decommission verb for a dead foreign owner, +/// a new id for a retired one. Neither advises a hand edit of the owner object as the first option. +TEST(CASServerRootClaim, RefusalsNameTheSupportedRecovery) +{ + auto b = std::make_shared(); + const Layout l("p"); + Ops ops(b); + claimOwnerOrThrow(ops.op, l, "r", UInt128(1), emptyCatalogObservation()); + DB::Cas::tests::expectThrowsCodeWithMessage(DB::ErrorCodes::CORRUPTED_DATA, "SYSTEM CAS DROP POOL MEMBER 'r'", [&] + { + claimOwnerOrThrow(ops.op, l, "r", UInt128(2), emptyCatalogObservation()); + }); + + mustCommit(ops.op.create(l.ownerKey("t"), encodeOwner(OwnerObject{ + .server_uuid = UInt128(1), + .retired_at_ms = 1752537600000ULL, + }), Retry::standard()), "tombstoned owner"); + try + { + claimOwnerOrThrow(ops.op, l, "t", UInt128(1), emptyCatalogObservation()); + FAIL() << "expected a tombstoned owner claim to fail closed"; + } + catch (const DB::Exception & e) + { + EXPECT_EQ(e.code(), DB::ErrorCodes::CORRUPTED_DATA); + EXPECT_NE(e.message().find("explicitly decommissioned"), String::npos) << e.message(); + EXPECT_NE(e.message().find("new "), String::npos) << e.message(); + EXPECT_EQ(e.message().find("clear the owner object"), String::npos) << e.message(); + } +} + TEST(CASServerRootEpoch, AllocatorIsMonotoneAndSurvivesMountConcept) { auto b = std::make_shared(); @@ -744,7 +799,11 @@ TEST(CASServerRootSafety, EveryCatalogLifecycleStateBlocksOwnerAndEpochRecreatio const ObserveRefCatalog observe = [catalog] { return catalog; }; Ops owner_ops(std::make_shared()); - EXPECT_THROW(claimOwnerOrThrow(owner_ops.op, layout, "root/x", UInt128{1}, observe), DB::Exception); + DB::Cas::tests::expectThrowsCodeWithMessage( + DB::ErrorCodes::CORRUPTED_DATA, "identity lost over existing data", [&] + { + claimOwnerOrThrow(owner_ops.op, layout, "root/x", UInt128{1}, observe); + }); EXPECT_FALSE(owner_ops.op.head(layout.ownerKey("root/x"), Retry::standard()).has_value()); Ops epoch_ops(std::make_shared()); @@ -773,9 +832,9 @@ TEST(CASServerRootSafety, OpaqueStreamAndStateDebrisAloneDoesNotBlockRecreation) mustCommit(ops.op.create(layout.refCkptKey(dead), "debris", Retry::standard()), "ckpt debris"); mustCommit(ops.op.create(layout.namespaceFileKey(dead, "f"), "debris", Retry::standard()), "ns-file debris"); - EXPECT_NO_THROW(claimOwnerOrThrow(ops.op, layout, "root/x", UInt128{1}, emptyCatalogObservation())); + EXPECT_NO_THROW(claimOwnerOrThrow(ops.op, layout, "rootx", UInt128{1}, emptyCatalogObservation())); EXPECT_EQ(allocateWriterEpoch( - ops.op, layout, "root/x", EpochMintPolicy::NormalMount, 0, emptyCatalogObservation()), 1u); + ops.op, layout, "rootx", EpochMintPolicy::NormalMount, 0, emptyCatalogObservation()), 1u); } TEST(CASServerRootSafety, ManifestAndLooseRootDebrisStillBlockRecreation) @@ -787,8 +846,11 @@ TEST(CASServerRootSafety, ManifestAndLooseRootDebrisStillBlockRecreation) { Ops ops(std::make_shared()); mustCommit(ops.op.create(key, "x", Retry::standard()), "blocking debris"); - EXPECT_THROW(claimOwnerOrThrow( - ops.op, layout, "root/x", UInt128{1}, emptyCatalogObservation()), DB::Exception); + DB::Cas::tests::expectThrowsCodeWithMessage( + DB::ErrorCodes::CORRUPTED_DATA, "identity lost over existing data", [&] + { + claimOwnerOrThrow(ops.op, layout, "root/x", UInt128{1}, emptyCatalogObservation()); + }); EXPECT_THROW(allocateWriterEpoch( ops.op, layout, "root/x", EpochMintPolicy::NormalMount, 0, emptyCatalogObservation()), DB::Exception); } @@ -802,7 +864,11 @@ TEST(CASServerRootSafety, UnreadableCatalogNeverFallsBackToPhysicalGuesses) { throw DB::Exception(DB::ErrorCodes::CORRUPTED_DATA, "injected unreadable catalog"); }; - EXPECT_THROW(claimOwnerOrThrow(ops.op, layout, "root/x", UInt128{1}, unreadable), DB::Exception); + DB::Cas::tests::expectThrowsCodeWithMessage( + DB::ErrorCodes::CORRUPTED_DATA, "injected unreadable catalog", [&] + { + claimOwnerOrThrow(ops.op, layout, "root/x", UInt128{1}, unreadable); + }); EXPECT_THROW(allocateWriterEpoch( ops.op, layout, "root/x", EpochMintPolicy::NormalMount, 0, unreadable), DB::Exception); EXPECT_FALSE(ops.op.head(layout.ownerKey("root/x"), Retry::standard()).has_value()); @@ -820,9 +886,9 @@ TEST(CASServerRootSafety, OwnerConflictRecomputesTheWholeEmptinessBundle) DB::Cas::tests::expectThrowsCodeWithMessage( DB::ErrorCodes::CORRUPTED_DATA, "newly visible owned work blocks recreation", - [&] { claimOwnerOrThrow(ops.op, layout, "root/x", UInt128{1}, emptyCatalogObservation()); }); + [&] { claimOwnerOrThrow(ops.op, layout, "rootx", UInt128{1}, emptyCatalogObservation()); }); EXPECT_TRUE(backend->fired); - EXPECT_FALSE(ops.op.head(layout.ownerKey("root/x"), Retry::standard()).has_value()); + EXPECT_FALSE(ops.op.head(layout.ownerKey("rootx"), Retry::standard()).has_value()); } TEST(CASServerRootSafety, EpochConflictRecomputesTheWholeEmptinessBundle) @@ -2487,9 +2553,9 @@ TEST(CASListMounts, ClassifiesEveryStateReadOnly) } } -/// A `srid` may itself contain `/` (e.g. `shard-01/replica-a` — legal per -/// `CASServerRootId.ValidationAcceptsCleanPathsRejectsBad`). Slicing the key by the last `/` before -/// the `/mount` suffix (as opposed to by `serverRootsPrefix()` length) truncates it to `replica-a`. +/// A pool may hold a mount slot whose `srid` contains `/`, written before `validateServerRootId` refused +/// one; `system.cas_mounts` must still name it whole. Slicing the key by the last `/` before the `/mount` +/// suffix (as opposed to by `serverRootsPrefix()` length) truncates it to `replica-a`. TEST(CASListMounts, NestedSridIsNotTruncated) { auto backend = std::make_shared(); diff --git a/src/Disks/tests/gtest_cas_s3_staging.cpp b/src/Disks/tests/gtest_cas_s3_staging.cpp index 74a82df51569e..42b320ef68ce7 100644 --- a/src/Disks/tests/gtest_cas_s3_staging.cpp +++ b/src/Disks/tests/gtest_cas_s3_staging.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -44,6 +45,7 @@ namespace DB::ErrorCodes { extern const int CORRUPTED_DATA; extern const int FILE_DOESNT_EXIST; + extern const int NETWORK_ERROR; } namespace @@ -100,6 +102,15 @@ class FakeNativeCopyObjectStorage final : public DB::LocalObjectStorage || (mode == DB::ObjectStorageCopyMode::NativeOnly && native_only_copy_supported); } + void removeObjectIfExists(const DB::StoredObject & object) override + { + if (throw_on_remove.load()) + throw DB::Exception(DB::ErrorCodes::NETWORK_ERROR, "injected delete failure for {}", object.remote_path); + DB::LocalObjectStorage::removeObjectIfExists(object); + } + + std::atomic throw_on_remove{false}; + private: const bool native_only_copy_supported; }; @@ -842,6 +853,29 @@ void writeThroughS3Transaction(DB::ContentAddressedTransaction & tx, const std:: buf->finalize(); } +/// Stages one blob through a real transaction, lets the transaction abort, and re-creates the exact key +/// the writer minted: the abort delete is best-effort, so a lost delete leaves the object at precisely +/// this key. Returns the key as the object storage sees it. +std::string leakWriterStagedObject(DB::IObjectStorage & object_storage, DB::ContentAddressedMetadataStorage & metadata_storage) +{ + std::string key; + { + auto tx = metadata_storage.createTransaction(); + auto & ca_tx = dynamic_cast(*tx); + writeThroughS3Transaction( + ca_tx, "a11/a11a11a1-1111-4111-8111-111111111111/all_1_1_0/data.bin", std::string(300, 'x')); + DB::RelativePathsWithMetadata staged; + object_storage.listObjects(metadata_storage.stagingKeyPrefix(), staged, /*max_keys=*/0); + if (staged.size() != 1) + throw std::runtime_error("expected exactly one staged object, got " + std::to_string(staged.size())); + key = staged.front()->relative_path; + } + auto buf = object_storage.writeObject(DB::StoredObject(key), DB::WriteMode::Rewrite); + buf->write("x", 1); + buf->finalize(); + return key; +} + } TEST(CASS3Staging, WritableS3StagingRequiresNativeOnlyCopy) @@ -933,6 +967,47 @@ TEST(CASS3Staging, ReadYourWritesReturnsStagedBytesFromS3StagingObject) EXPECT_EQ(got, payload); } +/// An aborted transaction deletes its own S3 staging object instead of leaving it for the next mount. +TEST(CASS3Staging, AbortDeletesS3Staging) +{ + auto object_storage = makeFakeNativeCopyStorage(/*native_only_copy_supported=*/true); + auto metadata_storage = makeS3StagingMetadataStorageForTest(object_storage, "mountAbort"); + metadata_storage->startup(); + + auto tx = metadata_storage->createTransaction(); + auto & ca_tx = dynamic_cast(*tx); + writeThroughS3Transaction( + ca_tx, "a11/a11a11a1-1111-4111-8111-111111111111/all_1_1_0/data.bin", std::string(300, 'x')); + + DB::RelativePathsWithMetadata staged; + object_storage->listObjects(metadata_storage->stagingKeyPrefix(), staged, /*max_keys=*/0); + ASSERT_EQ(staged.size(), 1u); + + tx.reset(); /// never committed + + DB::RelativePathsWithMetadata after; + object_storage->listObjects(metadata_storage->stagingKeyPrefix(), after, /*max_keys=*/0); + EXPECT_TRUE(after.empty()) << "an aborted transaction must not leave its S3 staging object behind"; +} + +/// Guard: the abort delete runs in a `noexcept` destructor, also after the storage was shut down with +/// the transaction still open (server shutdown). A failing delete must not escape it. +TEST(CASS3Staging, AbortAfterStorageShutdownDoesNotThrow) +{ + auto object_storage = makeFakeNativeCopyStorage(/*native_only_copy_supported=*/true); + auto metadata_storage = makeS3StagingMetadataStorageForTest(object_storage, "mountAbortLate"); + metadata_storage->startup(); + + auto tx = metadata_storage->createTransaction(); + auto & ca_tx = dynamic_cast(*tx); + writeThroughS3Transaction( + ca_tx, "a11/a11a11a1-1111-4111-8111-111111111111/all_1_1_0/data.bin", std::string(300, 'x')); + + metadata_storage->shutdown(); + object_storage->throw_on_remove = true; + EXPECT_NO_THROW(tx.reset()); +} + /// (c) `sweepOwnMountStaging` removes only objects under the given mount prefix and leaves a DIFFERENT /// mount's staging objects untouched (the lease-fence — `CASStagingSweeper.h`). TEST(CASStagingSweeper, RemovesOnlyObjectsUnderGivenMountPrefix) @@ -996,6 +1071,46 @@ TEST(CASS3Staging, GcBlobDiscoveryPrefixExcludesStagingObjects) EXPECT_FALSE(staging_key.starts_with(blobs_prefix)); } +/// Guard: the drain lists `/staging//` as a LOGICAL key and the backend applies the +/// local root itself, while the writer mints its keys through `physicalKey`. Local prefix: +/// `EmulatedSingleProcess` over a `LocalObjectStorage`, whose physical keys carry its root. +TEST(CASDecommission, StagingDrainRemovesWhatTheWriterStagedUnderTheLocalPrefix) +{ + auto object_storage = makeFakeNativeCopyStorage(/*native_only_copy_supported=*/true); + auto metadata_storage = makeS3StagingMetadataStorageForTest(object_storage, "victimLocal"); + metadata_storage->startup(); + const std::string leaked = leakWriterStagedObject(*object_storage, *metadata_storage); + metadata_storage->shutdown(); /// clean farewell: the member is dead and claimable without a wait + ASSERT_TRUE(object_storage->exists(DB::StoredObject(leaked))); + + auto backend = std::make_shared( + object_storage, DB::Cas::ObjectStorageBackend::Mode::EmulatedSingleProcess); + const DB::Cas::DecommissionReport report = DB::Cas::decommissionPoolMember( + backend, DB::Cas::PoolConfig{.pool_prefix = "pool", .server_root_id = "admin"}, "victimLocal"); + + EXPECT_EQ(report.staging_objects_removed, 1u) << "the drain did not see the writer's key space"; + EXPECT_FALSE(object_storage->exists(DB::StoredObject(leaked))); +} + +/// Guard: a staging object whose abort delete was lost is reclaimed by the next mount of the same +/// `server_root_id`, through the same prefix the writer minted it under. +TEST(CASS3Staging, MountStartSweepsALostAbortDelete) +{ + auto object_storage = makeFakeNativeCopyStorage(/*native_only_copy_supported=*/true); + std::string leaked; + { + auto first_mount = makeS3StagingMetadataStorageForTest(object_storage, "mountSweep"); + first_mount->startup(); + leaked = leakWriterStagedObject(*object_storage, *first_mount); + first_mount->shutdown(); + } + ASSERT_TRUE(object_storage->exists(DB::StoredObject(leaked))); + + auto second_mount = makeS3StagingMetadataStorageForTest(object_storage, "mountSweep"); + second_mount->startup(); + EXPECT_FALSE(object_storage->exists(DB::StoredObject(leaked))); +} + #if USE_AWS_S3 namespace @@ -1335,4 +1450,24 @@ TEST(CASS3Staging, GenerationBackendMayUseNativeOnlyCopy) EXPECT_EQ(staged.size(), 1u); } +/// Native prefix: keys pass through unchanged on both sides, so a `physicalKey` that started adding a +/// root for Native, or a drain that started adding one, would miss the writer's objects. +TEST(CASDecommission, StagingDrainRemovesWhatTheWriterStagedUnderTheNativePrefix) +{ + auto object_storage = makeFakeGenerationObjectStorageForTest(); + auto metadata_storage = makeS3StagingMetadataStorageForTest(object_storage, "victimNative"); + metadata_storage->startup(); + const std::string leaked = leakWriterStagedObject(*object_storage, *metadata_storage); + metadata_storage->shutdown(); + ASSERT_TRUE(object_storage->exists(DB::StoredObject(leaked))); + + auto backend = std::make_shared( + object_storage, DB::Cas::ObjectStorageBackend::Mode::Native); + const DB::Cas::DecommissionReport report = DB::Cas::decommissionPoolMember( + backend, DB::Cas::PoolConfig{.pool_prefix = "pool", .server_root_id = "admin"}, "victimNative"); + + EXPECT_EQ(report.staging_objects_removed, 1u) << "the drain did not see the writer's key space"; + EXPECT_FALSE(object_storage->exists(DB::StoredObject(leaked))); +} + #endif diff --git a/src/Parsers/ParserSystemQuery.cpp b/src/Parsers/ParserSystemQuery.cpp index ed841ef3bea1a..6b396578d9f93 100644 --- a/src/Parsers/ParserSystemQuery.cpp +++ b/src/Parsers/ParserSystemQuery.cpp @@ -505,7 +505,7 @@ bool ParserSystemQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, Expected & { /// SYSTEM CAS DROP POOL MEMBER FROM DISK [ON CLUSTER cluster]. /// Both the srid and the disk name are REQUIRED quoted string literals -- an srid is an - /// opaque server-root path (may contain '/'), not the bare identifier the sibling + /// opaque string, not the bare identifier the sibling /// CAS_* commands' disk TARGET accepts, so this does not go through /// parseQueryWithOnClusterAndTarget. ASTPtr ast; diff --git a/tests/integration/test_cas_drop_pool_member/test.py b/tests/integration/test_cas_drop_pool_member/test.py index 6ac60271348db..3db394e13c4dd 100644 --- a/tests/integration/test_cas_drop_pool_member/test.py +++ b/tests/integration/test_cas_drop_pool_member/test.py @@ -178,7 +178,7 @@ def test_drop_dead_pool_member_heals_the_pool(): # be live under its conservative safety margin. The destructive side being stricter is the # fail-close direction, so the table saying "not live" does not guarantee the command is # ready yet -- under sanitizer slowdowns the gap is wide enough to hit. Retry the command - # itself through the documented "wait for its lease to lapse" refusal, bounded. + # itself through its `alive or contended` refusal until the slot turns `fenced`, bounded. report_tsv = None for _ in range(90): try: @@ -190,7 +190,7 @@ def test_drop_dead_pool_member_heals_the_pool(): if "alive or contended" not in str(e): raise time.sleep(1.0) - assert report_tsv is not None, "decommission kept refusing: lease never lapsed within the bound" + assert report_tsv is not None, "decommission kept refusing as alive or contended: the slot never turned fenced within the bound" fields = report_tsv.split("\t") assert len(fields) == 10, report_tsv assert fields[0] == SRID2, report_tsv @@ -321,3 +321,13 @@ def test_drop_pool_member_rejected_on_readonly_disk(): "SYSTEM CAS DROP POOL MEMBER 'whatever' FROM DISK '{}'".format(RO_DISK) ) assert "read-only" in err, err + + +def test_drop_pool_member_refuses_id_with_slash(): + # The drain selects a member's objects by key prefix, so it refuses an id with '/'. + node1 = cluster.instances["node1"] + err = node1.query_and_get_error( + "SYSTEM CAS DROP POOL MEMBER 'node2/nested' FROM DISK '{}'".format(CA_DISK) + ) + assert "BAD_ARGUMENTS" in err, err + assert "contains '/'" in err, err