feat: Add optional external storage plugins with a pass-through example - #47
Open
brightsparc wants to merge 6 commits into
Open
feat: Add optional external storage plugins with a pass-through example#47brightsparc wants to merge 6 commits into
brightsparc wants to merge 6 commits into
Conversation
…nd ABI wart Four gaps against the repository's own rules, plus the lint gate. Principle VII names a protocol change without a docs/ROUNDTRIPS.md row as a tell, and that document's own scope line covers "the ObjectStore backends themselves". Adds the row: a decorator is in-process, so every existing budget is unchanged. Also states what the table cannot predict -- each operation crosses the boundary as one metadata frame plus one per MAX_FRAME of body, each a spawn_blocking hop on the same pool the control plane uses, and a decorator that calls a key service adds round trips whose budget belongs to its author. STORAGE_PLUGINS.md was reachable only from D42, not from the AGENTS.md section 0 document map that lists every other document with who should read it. export_plugin! requires the plugin's own manifest to depend on abi_stable: the macro expands #[abi_stable::export_root_module] through a bare path, and an attribute resolves against the plugin crate's root rather than this crate's pub use re-export, which every other path in the expansion goes through. The error points at the macro call site rather than the missing manifest entry. Two fixes were tried and neither works -- $crate is not resolved in attribute position, and attribute paths do not consult local use items -- so removing it means not applying export_root_module inside the macro at all. Documented rather than papered over. Adds a throughput probe alongside the existing latency table, which says it does not isolate the ABI's own cost. Running one encrypting workload in-process and again behind the plugin does isolate it: the boundary is not free (under the manual ABI a plugin doing nothing was slower than in-process AES-256-GCM), and the checked implementation is 25-30% faster because an owned RVec lets Bytes adopt the plugin's allocation rather than copying out of it. On this workload safety and speed agree. Also fixes a missing-backticks doc lint in walgit-cli that fails `cargo clippy --workspace --all-targets -- -D warnings` on this branch today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaoVaspfci2kQbzQJLTPd7
brightsparc
marked this pull request as ready for review
September 10, 2026 04:52
The overhead tables read as though the boundary is always paid. It is not: with [store.plugin] unset, open_store returns the backend store and no decorator is constructed, so none of the measured code is on the request path. DynStore is Arc<dyn ObjectStore> with or without the seam, so an undecorated store gains no indirection either. The residual cost of a plugin-free build is compile time and binary size for abi_stable, not latency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaoVaspfci2kQbzQJLTPd7
# Conflicts: # AGENTS.md # Cargo.lock # crates/walgit-cli/src/bundle_cmd.rs # docs/CONTRACT.md # justfile
Author
|
@tobi The simplification in #52–#58: one inventory, immutable packs, no second catalog is nice. Rebased onto main after #52–#58 by merging main into this branch (1706cae). The conflicts were all in the bundle removal's wake: bundle_cmd.rs is dropped (this PR's only change there was routing it through the plugin-aware open_store, and the file no longer exists), the D42 plugin decision now sits beside your new D47 in AGENTS.md, the [store.plugin] paragraph in CONTRACT.md precedes the post-bundle command line, and just test-plugin is appended to the reworked test recipe. Cargo.lock was regenerated by cargo. Workspace clippy and the native plugin boundary test are green on the merged head. |
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.
Adds a plugin seam so capabilities can be added to walgit's storage layer without forking it or changing the
ObjectStoretrait. An operator-installed shared library decorates the configured S3, GCS or memory store.With no plugin configured there is no overhead — not low, zero. Same stock binaries, same backend behaviour, same stored formats.
What this unlocks
ObjectStore, so anything expressible against that trait can be added downstream — encryption at rest, audit interception, compression, transparent double-writing during a bucket migration, extra caching — with no upstream change per capability..soalongside it — no branch to rebase, no private build to keep current.This PR adds the seam, not any particular capability. The only functional example in-tree is a no-op pass-through, and nothing here adds encryption, tenant identity, key-management policy, or Azure Blob support.
How it works
One CLI store constructor (
open_store) applies the decorator across serving, maintenance and administrative commands. The factory receives the configured backend with its global prefix already applied once, so the decorator only ever sees logical keys. A downstream crate returns anyObjectStoreviawalgit_store_plugin::export_plugin!(factory).Built on abi_stable 0.11.3 — checked root modules,
sabi_traitendpoints, ownedRBox/RVec/RString, following Rotel's Rust processor SDK and its async bridge.Boundary details and constraints
crate-type = ["cdylib"]plus a directabi_stabledependency.Native plugins are trusted code. ABI checks are not a sandbox — aborts or panicking destructors can still take down the process.
Rejected combinations. Bucket mounts are refused alongside plugins, because a mount bypasses the decorator entirely. A transforming plugin must also disable or correctly implement signed URLs, acceleration and native composition; the SDK cannot infer a plugin's transformation policy. Known MIME hints are preserved, unknown advisory ones omitted.
Cost
Unconfigured: zero, structurally.
open_storereturns the backend store itself when[store.plugin]is unset — no decorator is constructed, so none of the code below is on the request path.DynStoreisArc<dyn ObjectStore>with or without this change, so an undecorated store gains no indirection either, and there is no per-operation branch to skip. Nothing in the tables below is paid by a deployment that doesn't load a plugin; the only residual cost of the seam existing is compile time and binary size forabi_stable.Loaded: measured below. The hand-written ABI prototype (raw pointer handles, vtables, release callbacks) was replaced by the checked one. It also got faster, so the two probes are recorded for the record rather than as an argument.
Interface latency — macOS arm64, test-profile
MemoryStorefull GETs, three alternating runs, median of per-run means:This does not isolate the ABI library's cost — the new implementation also removes a receiver-side frame copy.
Encryption overhead — this one does isolate the boundary, running the same AES-256-GCM workload in-process and again behind the plugin. Linux x86-64, release profile,
MemoryStore, 16 MiB objects read in 1 MiBBLOCK_SIZE-aligned ranges (a pack's real read shape), best-of-4:Two things the latency table cannot show:
RVecletsBytesadopt the plugin's allocation instead of copying out of it.The cost is per byte, not per request — a whole-object read matches sixteen separate 1 MiB reads, and raising
MAX_FRAMEfrom 1 MiB to 8 MiB measured worse. It is not tunable by framing.MemoryStorewith no KMS. A decorator that calls out to a key service pays round trips none of this measures. No network, Git or KMS is in any figure above; these are not production throughput claims.Validation
Checked at
2e1f168on macOS arm64, Rust 1.97.1, Git 2.55.0:just web-buildjust warningsjust testcargo test -p walgit-server --test simjust test-s3)just clippyclippy::cast_losslesserrors atwalgit-wal/src/registry.rs:494–495on macOS. This file is identical to the PR base.just e2efetch_from_front_that_serves_the_base_remotelyfailed after a commit-graph lock conflict lefthas_commit_graph=false; passed when run alone.just ciThe e2e failure is pre-existing and already documented in the base. Its test and harness are unchanged and use the in-memory store with no plugin. The isolated pass is diagnostic, not a substitute for a green full run. No assertions were relaxed and no failing checks suppressed.
Live GCS contracts and the ignored benchmark/soak tiers were not run — the GCS cases in the S3 test binary skipped because no GCS bucket was supplied.
Since the latest commits (docs + lint fix):
cargo clippy --workspace --all-targets -- -D warningsnow exits 0 on Linux x86-64. The base failed it on a missing-backticks doc lint inwalgit-cli. The macOScast_losslesserrors above are base-identical and were not re-checked on macOS.Test coverage
The native-library contract covers streaming and file uploads, ranges, conditional reads/writes/deletes, CAS races, listing, single prefix application, interrupted uploads, and response lifetime after the store is dropped. A deliberately incompatible module verifies layout rejection before the storage factory is invoked; missing libraries and failed factories also fail.
just testbuilds and runs these native fixtures; the timing probes stay opt-in (passthrough_overhead,--ignored --nocapture --test-threads=1withWALGIT_TEST_PLUGINset).Docs
docs/STORAGE_PLUGINS.md— the interface and the adapter's obligations.docs/ROUNDTRIPS.md— the round-trip budget row for a decorated store.