[PR] feat: add conditional deletes (`ObjectStore::delete_opts`) [arrow-rs-object-store]

["rossoha (via GitHub)" <gi...@apache.org>]

rossoha opened a new pull request, #864:
URL: https://github.com/apache/arrow-rs-object-store/pull/864

   # feat: add conditional deletes (`ObjectStore::delete_opts`)
   
   Addresses #298.
   
   ## Summary
   
   Adds a backend-neutral API for compare-and-swap deletes: an object is deleted
   only if its current version matches the version the caller last observed.
   
   ```text
   delete object
   ONLY IF
   current object version == expected version
   ```
   
   Conditional delete provides optimistic concurrency control. A backend that
   supports the supplied precondition performs the check **atomically with the
   deletion**, as part of a single conditional request. Implementations must not
   emulate it with a separate metadata read followed by an unconditional delete —
   that sequence has a race between the two requests.
   
   ## API
   
   ```rust
   pub struct DeleteOptions {
       /// If set, the object is deleted only if its current version matches
       /// otherwise returning `Error::Precondition`
       pub precondition: Option<UpdateVersion>,
       pub extensions: Extensions,
   }
   
   trait ObjectStore {
       async fn delete_opts(&self, location: &Path, options: DeleteOptions) -> Result<()>;
   }
   ```
   
   Usage:
   
   ```rust
   let meta = store.head(&path).await?;
   let options = DeleteOptions::new().with_precondition(Some(UpdateVersion::from(meta)));
   store.delete_opts(&path, options).await?; // Err(Precondition) if it changed
   ```
   
   `UpdateVersion` (the existing `e_tag` + `version` identity, already used by
   `PutMode::Update`) is reused rather than introducing a second version model; each
   backend picks the field its native mechanism needs. `From<ObjectMeta> for
   UpdateVersion` is added for ergonomics.
   
   ### `delete` / `delete_opts` / `delete_stream`
   
   - `delete_opts` is a **provided** trait method, so existing implementations keep
     compiling. Its default returns `Error::NotSupported` when a precondition is
     supplied, and otherwise behaves exactly like today's single-object delete
     (drives `delete_stream` with one location).
   - `ObjectStoreExt::delete` is now `delete_opts(location, DeleteOptions::default())`,
     so unconditionally deleting is unchanged for every backend, including the S3
     bulk `DeleteObjects` path.
   - `delete_stream` is deliberately **unchanged**: it is a bulk API, whereas
     conditional delete is per-object. Bulk conditional deletes are out of scope
     here and can be added later without breaking this design.
   
   ## Backends
   
   | Backend | Mechanism | Status |
   | --- | --- | --- |
   | GCS | `x-goog-if-generation-match` with `ObjectMeta::version` (generation) | implemented |
   | S3 | `If-Match` with the ETag, reusing the existing `S3ConditionalPut` gate | implemented |
   | Azure | `If-Match` with the ETag on `Delete Blob` | implemented |
   | InMemory | ETag compared and the entry removed under the same lock | implemented |
   | Local FS | not atomic, returns `NotSupported`; unconditional delete unchanged | not supported |
   | HTTP/WebDAV | default implementation / `NotImplemented` | not supported |
   
   Errors reuse the existing taxonomy: a failed precondition surfaces as
   `Error::Precondition` (the status mapping for `412` already exists), so callers
   can distinguish "object does not exist" (`NotFound`) from "object changed"
   (`Precondition`). Backends that cannot evaluate the precondition return
   `NotSupported`/`NotImplemented` instead of silently ignoring it.
   
   For S3, some S3-compatible endpoints do not implement `If-Match` on
   `DeleteObject` and answer `501 Not Implemented`; that is surfaced as
   `Error::NotSupported` so callers can fall back to an unconditional delete, rather
   than reporting an opaque transport error. `S3ConditionalPut::Disabled` also
   disables conditional deletes, consistent with conditional puts.
   
   ## Performance
   
   No additional round trips. `delete(path)` remains one request (bulk
   `DeleteObjects` on S3, since the existing single-object behavior is preserved),
   and a conditional delete is a single conditional `DELETE`, never `HEAD` + `DELETE`.
   
   ## Compatibility
   
   Additive: new types plus a defaulted trait method, no signature changes, no
   behavior change for existing paths. This makes it suitable for a minor release
   (`0.14.x`) rather than requiring `0.15.0`.
   
   One practical consequence: wrapper stores that use
   `#[deny(clippy::missing_trait_methods)]` — the pattern recommended in this
   crate's own docs — must forward `delete_opts` explicitly once the default exists.
   This PR does that for `ChunkedStore`, `LimitStore`, `PrefixStore` and
   `ThrottledStore`.
   
   ## Testing
   
   - Trait-level integration test (`integration::conditional_delete`): unconditional
     delete, correct-version delete, stale-version delete returning `Precondition`
     with the newer object and its content untouched, and a `NotSupported`/`NotImplemented`
     path that asserts the object is left alone while ordinary deletes keep working.
     Wired into the in-memory, local filesystem, `Box`/`Arc`, S3, Azure and GCS suites.
   - Request-shape unit tests with the mock HTTP server for GCS, S3 and Azure:
     exactly one `DELETE` carrying the expected precondition header (a second queued
     handler fails the test, proving there is no preceding `HEAD`), `412` surfacing
     as `Error::Precondition`, and — for S3 — that an unconditional delete still
     uses bulk `DeleteObjects`.
   - A stateful stub reproducing GCS semantics (stale generation → `412`, matching
     generation → `204` and object gone).
   - Verified live against Azurite (Azure): stale version → `412 Precondition
     Failed`, newer object preserved, matching version deletes. LocalStack reports
     `501` for `If-Match` on `DELETE` and the test skips via `NotSupported`.
   
   ## Notes for reviewers
   
   1. The fake GCS server used by the default integration job silently **ignores**
      `ifGenerationMatch` on delete, so the GCS conditional-delete scenario only
      runs when pointed at the real GCS endpoint (same gating as the existing
      `ifGenerationMatch`-dependent tests).
   2. Open question: keep S3 conditional deletes gated behind the existing
      `S3ConditionalPut`, or introduce a separate knob?
   3. Open question: is `DeleteOptions::precondition` the preferred shape, or would
      maintainers prefer a `DeleteMode`-style enum (as with `PutMode`/`CopyMode`)?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: add conditional deletes (`ObjectStore::delete_opts`) [arrow-rs-object-store]

["rossoha (via GitHub)" <gi...@apache.org>]

rossoha commented on PR #864:
URL: https://github.com/apache/arrow-rs-object-store/pull/864#issuecomment-5855235000

   Context for reviewers — this supersedes my earlier #862 (same branch, same diff,
   closed right after opening while the description was being rewritten) and it
   overlaps #610.
   
   Design-wise it matches the direction already discussed on #610 and in
   [tomsanbear's rebase][rebase]: a unary `delete_opts` alongside the batch
   `delete_stream` (as @tustvold proposed), with `UpdateVersion` as the version
   identity so each backend can pick the field its native precondition needs
   (GCS generation, S3/Azure ETag). Concretely, compared to #610's current head
   (`DeleteOptions { if_match: Option<String> }`, which cannot express a GCS
   generation):
   
   - `DeleteOptions::precondition: Option<UpdateVersion>` — happy to rename this to
     `condition` to match [tomsanbear's rebase][rebase] if that shape is preferred.
   - `InMemory` performs the check and the removal under the same write lock, so it
     is genuinely atomic rather than check-then-delete.
   - S3: an endpoint answering `501 Not Implemented` for `If-Match` on
     `DeleteObject` is surfaced as `Error::NotSupported` (verified: LocalStack
     responds `501`, so the test skips instead of failing), and conditional deletes
     honour the existing `S3ConditionalPut` gate. A test pins that the unconditional
     single-object path still uses bulk `DeleteObjects`.
   - The trait docs state explicitly that a supporting backend must evaluate the
     precondition atomically and must not emulate it with `HEAD` + `DELETE`.
   - Evidence beyond request-shape tests: live runs against Azurite (stale version →
     `412` → `Error::Precondition`, newer object preserved, matching version deletes)
     and the finding that the fake GCS server used by the default integration job
     *silently ignores* `ifGenerationMatch` on delete, so the GCS scenario runs only
     against the real endpoint.
   
   Happy to fold any of this into #610 or to align naming — just say which.
   [rebase]: https://github.com/tomsanbear/arrow-rs-object-store/tree/feat/conditional-deletes
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org