-
Notifications
You must be signed in to change notification settings - Fork 0
feat(parquet): RFC0013 red — store module skeleton + §5 ignored stubs #230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0ce8270
feat(parquet): rfc0013 red — store module skeleton + §5 ignored stubs
jensholdgaard abcfd3a
feat(parquet): rfc0013 store.s3 returns StoreError::Unimplemented, no…
jensholdgaard 47e2fd7
test(parquet): rfc0013 stubs use the Scenario doc-comment convention …
jensholdgaard eabfcd5
test(parquet): add 'See …§5' line to rfc0013 scenario docs (verificat…
jensholdgaard 82231c3
feat(parquet): document rfc0013 store red caveats (allow rationale, p…
jensholdgaard File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| //! Object-storage backend (RFC 0013) — the seam behind the writer, reader, | ||
| //! compaction, and audit sink so the RFC 0005 data + audit Parquet and the | ||
| //! RFC 0009 manifest live on local disk (dev/test) or an S3-compatible | ||
| //! bucket (production), without changing the on-disk layout. | ||
| //! | ||
| //! **Status: `red` (RFC 0013).** This is the skeleton the `green` work fills: | ||
| //! the [`Store`] type and its constructors exist and the `LocalFileSystem` | ||
| //! backend is wired, but the S3 backend, the conditional-PUT atomic publish | ||
| //! (RFC0013.3/.4), and the migration of the writer/reader/compaction/audit | ||
| //! consumers from `bucket_root: &Path` onto [`Store`] are not done. The §5 | ||
| //! acceptance scenarios are encoded as `#[ignore]`d stubs in | ||
| //! `tests/rfc0013_object_store.rs` and turn green as the backend lands. | ||
| //! | ||
| //! Per RFC 0013 §3.7 the backend is a **module here in `ourios-parquet`** | ||
| //! (not a new crate): `ourios-querier`, `-ingester`, and `-server` already | ||
| //! depend on this crate, so the type is visible to every storage consumer. | ||
|
|
||
| use std::sync::Arc; | ||
|
|
||
| use object_store::ObjectStore; | ||
| use object_store::local::LocalFileSystem; | ||
| use object_store::path::Path as ObjectPath; | ||
|
|
||
| /// A handle to the object store backing a tenant store's Parquet + manifest | ||
| /// objects, addressed by key under `prefix`. Wraps an [`ObjectStore`] so the | ||
| /// same code path targets `LocalFileSystem` or `AmazonS3` / S3-compatible. | ||
| /// | ||
| /// **`red` caveat:** `prefix` is reserved and currently always empty, and | ||
| /// [`Store::object_store`] returns the raw backend with **no prefix | ||
| /// scoping**. Per-tenant/prefix isolation (RFC0013.5) is wired at `green` — | ||
| /// do **not** assume this type enforces isolation yet. | ||
| #[derive(Clone)] | ||
| pub struct Store { | ||
| inner: Arc<dyn ObjectStore>, | ||
| /// Reserved key prefix (the store root). Always empty at `red`; honoured | ||
| /// once the consumers migrate onto [`Store`] at `green`. | ||
| prefix: ObjectPath, | ||
| } | ||
|
|
||
| /// Configuration for the S3 / S3-compatible backend (RFC0013.7). Populated | ||
| /// from RFC 0004 config at `green`; a placeholder here so the `red` | ||
| /// constructor signature is stable. | ||
| /// | ||
| /// `Default` is a `red` placeholder only — it yields an **empty `bucket`**, | ||
| /// which is not valid; callers must set a non-empty `bucket` (the `green` | ||
| /// `s3()` will reject an empty one). | ||
| #[derive(Debug, Clone, Default)] | ||
| #[non_exhaustive] | ||
| pub struct S3Config { | ||
| /// Bucket name (required; the empty `Default` is a `red` placeholder). | ||
| pub bucket: String, | ||
| /// Optional endpoint override for S3-compatible stores (`MinIO`, R2, …). | ||
| pub endpoint: Option<String>, | ||
| /// Region (AWS) — ignored by some S3-compatible stores. | ||
| pub region: Option<String>, | ||
| /// Key prefix within the bucket (the store root). | ||
| pub prefix: Option<String>, | ||
| } | ||
|
|
||
| /// Errors from constructing or addressing a [`Store`]. | ||
| #[derive(Debug)] | ||
| #[non_exhaustive] | ||
| pub enum StoreError { | ||
| /// Backend construction failed (bad root, credentials, endpoint, …). | ||
| Backend(object_store::Error), | ||
| /// A backend constructor not yet implemented at this RFC 0013 stage. | ||
| /// Returned (rather than panicking) so an accidental call fails | ||
| /// gracefully while `red`. | ||
| Unimplemented(&'static str), | ||
| } | ||
|
|
||
| impl std::fmt::Display for StoreError { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| match self { | ||
| Self::Backend(e) => write!(f, "object-store backend: {e}"), | ||
| Self::Unimplemented(what) => write!(f, "not implemented (RFC 0013 red): {what}"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl std::error::Error for StoreError { | ||
| fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { | ||
| match self { | ||
| Self::Backend(e) => Some(e), | ||
| Self::Unimplemented(_) => None, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Store { | ||
| /// Local-filesystem backend rooted at `root` (dev / test / CI). Preserves | ||
| /// today's on-disk layout — the RFC 0005 Hive keys become paths under | ||
| /// `root`. | ||
| /// | ||
| /// # Errors | ||
| /// [`StoreError::Backend`] if `root` cannot be opened as an | ||
| /// `object_store` `LocalFileSystem` (e.g. it does not exist). | ||
| pub fn local(root: impl AsRef<std::path::Path>) -> Result<Self, StoreError> { | ||
| let fs = LocalFileSystem::new_with_prefix(root).map_err(StoreError::Backend)?; | ||
| Ok(Self { | ||
| inner: Arc::new(fs), | ||
| prefix: ObjectPath::default(), | ||
| }) | ||
| } | ||
|
|
||
| /// S3 / S3-compatible backend (RFC0013.1/.4/.7). | ||
| /// | ||
| /// `red`: not yet built — the `green` implementation constructs an | ||
| /// `object_store::aws::AmazonS3` (behind the `aws` feature) from `cfg` | ||
| /// and the RFC 0004 credentials. | ||
| /// | ||
| /// # Errors | ||
| /// At `red`, always [`StoreError::Unimplemented`] — returned rather than | ||
| /// panicking so an accidental call (e.g. from another workspace crate) | ||
| /// fails gracefully. At `green` this becomes [`StoreError::Backend`] if | ||
| /// the `object_store` `AmazonS3` backend cannot be constructed (bad | ||
| /// endpoint, credentials, or bucket). | ||
| // `red` stub: `cfg` is unused until the `green` AmazonS3 impl consumes | ||
| // it (`needless_pass_by_value` fires because we never read it). The | ||
| // signature is fixed now so consumers can be written against it. | ||
| #[allow(clippy::needless_pass_by_value, unused_variables)] | ||
| pub fn s3(cfg: S3Config) -> Result<Self, StoreError> { | ||
| // RFC0013 green: build AmazonS3 from cfg + RFC 0004 creds. | ||
| Err(StoreError::Unimplemented( | ||
| "RFC0013 green: AmazonS3 / S3-compatible backend", | ||
| )) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /// The underlying [`ObjectStore`], for handing to `DataFusion`'s table | ||
| /// providers on the read path (RFC 0013 §2.2 — the querier registers the | ||
| /// same store rather than local file paths). | ||
| #[must_use] | ||
| pub fn object_store(&self) -> Arc<dyn ObjectStore> { | ||
| Arc::clone(&self.inner) | ||
| } | ||
|
|
||
| /// The store's root key prefix. | ||
| #[must_use] | ||
| pub fn prefix(&self) -> &ObjectPath { | ||
| &self.prefix | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| //! RFC 0013 — object-storage backend acceptance scenarios (§5). | ||
| //! | ||
| //! **Status: `red`.** These are the failing stubs that drive the `green` | ||
| //! implementation: each encodes one RFC0013.§5 scenario and currently | ||
| //! `todo!()`s. They are `#[ignore]`d so the default `cargo test` (and CI) | ||
| //! stays green while the backend is built — `green` replaces each body with | ||
| //! a real assertion and removes the `#[ignore]`. The S3-backed scenarios run | ||
| //! against a MinIO/localstack container (`testcontainers`); the local-backend | ||
| //! scenarios re-run the RFC 0005 / 0009 contract through `Store`. | ||
| //! | ||
| //! See `docs/rfcs/0013-object-storage.md` §5/§6. | ||
|
|
||
| // Anchor the stubs to the public surface so they fail to compile if the | ||
| // `Store` seam is removed out from under them. | ||
| use ourios_parquet::{S3Config, Store}; | ||
|
|
||
| /// Scenario RFC0013.1 — a `MinedRecord` batch written and read through the `AmazonS3` | ||
| /// backend recovers byte-for-byte against the local backend. | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.1 — red until the S3 backend + writer/reader migration land"] | ||
| fn rfc0013_1_round_trip_through_s3_backend() { | ||
| let _ = Store::s3; | ||
| todo!("RFC0013.1: S3 round-trip == local round-trip (MinIO testcontainer)") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.2 — the existing RFC 0005 / 0009 suites pass unchanged against the | ||
| /// `LocalFileSystem` backend after the seam refactor. | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.2 — red until the consumers run through Store"] | ||
| fn rfc0013_2_local_backend_regresses_nothing() { | ||
| todo!("RFC0013.2: RFC0005/0009 suites green via the LocalFileSystem Store") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.3 — two `compact_partition` runs racing on one partition: exactly | ||
| /// one manifest generation wins; no torn / doubled / missing rows. | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.3 — red until conditional-PUT atomic publish lands"] | ||
| fn rfc0013_3_atomic_publish_under_contention() { | ||
| todo!("RFC0013.3: exactly-one-wins under concurrent publishers") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.4 — generation publish uses conditional PUT (`PutMode::Create` / | ||
| /// `Update{ETag}`) with no `rename` dependency. | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.4 — red until the manifest swap uses conditional PUT"] | ||
| fn rfc0013_4_manifest_swap_via_conditional_put() { | ||
| todo!("RFC0013.4: publish path uses PutMode::Create/Update, never rename") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.5 — operations in tenant X's context address only X's key | ||
| /// sub-prefix; no read/write touches tenant Y's keys (`CLAUDE.md` §3.7). | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.5 — red until tenant key-prefix scoping is wired"] | ||
| fn rfc0013_5_tenant_isolation_across_prefix() { | ||
| todo!("RFC0013.5: no cross-tenant key access") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.6 — with an object-storage backend, only data/audit/manifest | ||
| /// objects reach the store; the WAL stays on local disk (`CLAUDE.md` §3.4). | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.6 — red until the server wires the object-store backend"] | ||
| fn rfc0013_6_wal_stays_local() { | ||
| let _ = S3Config::default(); | ||
| todo!("RFC0013.6: WAL frames local; only Parquet/manifest in the store") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.7 — an S3-compatible store (`MinIO`) configured via an endpoint | ||
| /// override (RFC 0004) reads and writes exactly as AWS S3. | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.7 — red until the S3 backend honours endpoint overrides"] | ||
| fn rfc0013_7_s3_compatible_endpoint_via_override() { | ||
| todo!("RFC0013.7: MinIO endpoint override works like AWS S3") | ||
| } | ||
|
|
||
| /// Scenario RFC0013.8 — the RFC 0005 §3.9 reader forward-compat contract (absent | ||
| /// columns default, unknown columns ignored) holds over the object store. | ||
|
Copilot marked this conversation as resolved.
|
||
| /// See `docs/rfcs/0013-object-storage.md` §5. | ||
| #[test] | ||
| #[ignore = "RFC0013.8 — red until the reader resolves objects through Store"] | ||
| fn rfc0013_8_reader_forward_compat_over_store() { | ||
| todo!("RFC0013.8: RFC0005 §3.9 holds reading through the object store") | ||
| } | ||
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.