Skip to content

remote_store: real object-store backend (S3/MinIO) with upload+cache split flow #123

Description

@RamakrishnaChilaka

Problem

StorageManager today is hardcoded to object_store::local::LocalFileSystem, and the remote_store engine's publish and search paths bypass the ObjectStore trait for bulk data, using raw POSIX calls (std::fs::rename, std::fs::read_dir) directly on the storage root. This means:

  • The object store root must be a local filesystem.
  • We cannot meaningfully test against MinIO, RustFS, S3, or any other real object store today.
  • Manifest writes go through object_store but split bundles do not, which defeats the abstraction.

Current code bindings

  • src/storage/mod.rs:27,45 — local: Arc<LocalFileSystem> / LocalFileSystem::new_with_prefix
  • src/engine/remote_store.rs:312 — std::fs::create_dir_all(&staging_for_build)
  • src/engine/remote_store.rs:314 — HotEngine::new_with_mappings opens Tantivy directly on a filesystem path
  • src/engine/remote_store.rs:361 — std::fs::rename(&staging_dir, &split_dir) for the atomic publish
  • src/engine/remote_store.rs:439 — std::fs::read_dir on the splits dir at search time
  • src/engine/remote_store.rs:184 — search reopens HotEngine on a filesystem path
  • Manifests store "checksum":"sha256:pending:<split_id>" — checksum is never actually computed.

Proposed work

  1. Add S3 backend in StorageManager::new, switching on URL scheme (s3://bucket/prefix, file:///path). Pick it up from AppConfig (endpoint, bucket, region, access key, path-style flag).
  2. Refactor publish path to: build split on local disk → upload bundle (per-file or tar) via ObjectStore::put → compute real checksum → append manifest via existing append_split_and_publish. Remove the std::fs::rename assumption.
  3. Refactor search path to: download + cache split bundles locally on first read (under e.g. <data_dir>/_remote_store_cache/), open Tantivy on the cache dir. Verify against manifest checksum on cache miss.
  4. Replace the sha256:pending:<split_id> placeholder with a real sha256 over the uploaded bundle before publish.
  5. Add integration tests backed by a MinIO container (e.g. testcontainers crate) covering: publish → search, restart-persistence, checksum mismatch rejection, generation monotonicity under concurrent publishes.

Out of scope for this issue

  • Cross-process publish coordination (today we rely on an in-process publish_lock; cross-process is a separate tracked item).
  • Janitor / split GC for orphaned uploads.
  • Writes to remote_store indices beyond the publish API (still 501).

Why file this separately

PR #121 delivers a real, mergeable slice — local object_store-backed manifests + publish API + schema_hash validation + the _remote_store orphan-cleanup fix, all live-tested on dev_cluster_release.sh. The S3 backend work is larger (bundle upload, cache, real checksums, config surface) and deserves its own PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions