Skip to content

fix: ledger-cache deadlock, MinIO path-style, and an embeddable Raft node - #1680

Merged
bplatz merged 7 commits into
mainfrom
fix/ledger-manager-lock-order-and-minio
Aug 26, 2026
Merged

fix: ledger-cache deadlock, MinIO path-style, and an embeddable Raft node#1680
bplatz merged 7 commits into
mainfrom
fix/ledger-manager-lock-order-and-minio

Conversation

@bplatz

@bplatz bplatz commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Five changes that came out of reviewing what an embedding process needs to run the Raft data plane without fluree-server. Two are bug fixes with live reports behind them; one is the embedding itself; two are small hardening and doc corrections.

Stacked on #1679.

ledger_cached could hang for the duration of an unrelated write

Two LedgerManager readers — try_running_attachment_events and get_loaded_view — called handle.snapshot() while still holding the global entries read guard. snapshot() runs compact_if_needed, which can take the handle's state write lock, and a transaction holds that lock for its whole stage-and-commit. So the indexer's attachment provider parked on a busy ledger's state lock with entries held; the next entries.write() — a cold load of any ledger — queued behind it, and because tokio's RwLock is write-fair, every later entries.read() queued too. ledger_cached on a ledger with nothing to do with the transaction blocked until it finished, with no timeout anywhere in the path. This matches a live report of a write racing an unrelated ledger's bootstrap.

The file already states the invariant and follows it in current_t and notify; both offenders now go through one helper that clones the handle out and drops the guard before awaiting. The regression test holds a write lock on one ledger, parks both readers against it, and asserts an unrelated ledger stays reachable within 500 ms. Reintroducing either held guard alone fails it.

MinIO did not work

The S3 endpoint override was public and documented with a MinIO example, but path-style addressing was not reachable anywhere outside fluree-db-iceberg. With only endpoint_url set the SDK emits http://bucket.minio:9000/key, which a plain MinIO without wildcard bucket-subdomain DNS rejects.

S3Config::force_path_style, the s3ForcePathStyle JSON-LD key, and FlureeBuilder::s3_force_path_style for embedders. Unset leaves the SDK default untouched. The test observes the URL shape directly by presigning a request — no network — and checks both directions.

The Raft nameservice node is now embeddable

Everything a process needs already existed, but fluree-db-server/src/raft.rs held half of it and ~180 lines of FlureeServerBuilder::build held the rest. An embedder had to re-derive both from the server's source.

raft.rs moves to fluree-db-consensus::raft::integration verbatim. It never referenced a server type — the one crate:: in the file was inside its own tests — so it compiled in its new home with zero changes. The assembly becomes raft::embedded::EmbeddedRaftNode::attach: committer, ledger-cache watermark hookup, worker supervisor, leader watcher, release task, owned together so they shut down in the order that keeps the content store consistent (workers, leader tasks, Raft core, release drain). The background indexer stays a host contribution because this crate does not depend on fluree-db-indexer. Fluree::default_index_config is public so an embedder hands the workers the engine's own thresholds.

The one thing an embedder must get right is the write seam: fluree-db-api sits below this crate and cannot name Committer, so Fluree::transact on a Raft-mode engine still writes locally. Writes go through EmbeddedRaftNode::committer. This is documented at the module, in the crate map, and in the operations guide.

Validated two ways. The server itself now builds through EmbeddedRaftNode, so the nine-test cluster suite exercises exactly the code an embedder calls. And tests/it_embedded_node.rs stands a node up with no server dependency at all, mounts the routers at a host-chosen prefix, writes through the committer, and checks the Raft log advanced and that the replicated head and the engine's cache agree on the committed t.

Staging-file names were unique per host, not across hosts

FileStorage stages a write as <file>.<pid>.<seq>.tmp. Two nodes sharing a content store over NFS can have the same pid and both start the sequence at zero — and under Raft every node writes the shared store, since whichever node owns a branch stages its commit blob there. The collision is benign (the address is the content hash; both writers hold identical bytes) but the loser's rename fails and reports a spurious write error.

A 64-bit random token drawn once per process now sits in the name. Not a node id: that would plumb cluster identity down into fluree-db-core, which has no business knowing about it, and the token covers the accidental two-servers-one-mount case too. The test re-executes the test binary as a child process and asserts the token component differs — comparing whole names would pass against the old code, since two pids on one host already differ.

The docs said commit staging was leader-only

Both the operations guide and the design doc said so in three places. The code does the opposite, on purpose: every node runs a worker supervisor, each branch is assigned to one node by rendezvous hashing, and the owning node — routinely a follower — stages the commit, writes the blob straight to the shared store, and ferries only the CID to the leader. That is the design working as intended: the log carries a reference, never the bytes, and distributing the blob-writing half across nodes is what lets that scale. The docs now say so, and say that the shared store must be reachable read-write from every node.

Also adds the ledger_exists integration test the API never had. It pins that create → commit → exists returns true in both id forms on file storage, and that a malformed id is an Err rather than false — the trap that turns an existing ledger into a phantom "missing" one under .unwrap_or(false), and the most plausible explanation for a report that it returned false.

Not in this PR

Migrating RaftIntegration::bootstrap onto RaftGroup::bootstrap (it would get the catch-up payload cap and a shared HTTP client for free, at the cost of a storage-layout shim for deployed clusters), and FlureeBuilder::with_committer (needs a dependency inversion). Neither blocks embedding.

bplatz added 5 commits August 23, 2026 07:26
Two `LedgerManager` readers — `try_running_attachment_events` and
`get_loaded_view` — called `handle.snapshot()` while still holding the
global `entries` read guard. `snapshot()` runs `compact_if_needed`,
which can take the handle's `state` write lock, and a transaction holds
that lock for its whole stage-and-commit.

So a background reader (the indexer's attachment provider is the caller
of both) parked on a busy ledger's state lock with `entries` held. The
next `entries.write()` — a cold load of any ledger at all — queued
behind it, and because tokio's `RwLock` is write-fair, every later
`entries.read()` queued too. `ledger_cached` on a ledger with nothing
to do with the transaction blocked until that transaction finished,
with no timeout anywhere in the path. Under a long write this is
indistinguishable from a hang, and it matches a live report of a write
racing an unrelated ledger's bootstrap.

The file already states the invariant and already follows it in
`current_t` and `notify`: clone the handle out, drop the guard, then
await. Both offenders now go through one helper that does exactly that.

The regression test holds a write lock on one ledger, parks both readers
against it, and asserts an unrelated ledger can still be inserted and
read within 500 ms. Reintroducing the held guard in either reader alone
fails it.
The S3 endpoint override was public — `FlureeBuilder::s3`, the
`s3Endpoint` JSON-LD key, documented with a MinIO example — but
path-style addressing was not reachable anywhere outside
`fluree-db-iceberg`. With only `endpoint_url` set the SDK still emits
virtual-hosted URLs (`http://bucket.minio:9000/key`), which a plain
MinIO without wildcard bucket-subdomain DNS rejects. The documented
MinIO example did not work.

Adds `force_path_style: Option<bool>` to `S3Config`, applied beside the
endpoint override, and threads it through the connection config
(`s3ForcePathStyle`), the JSON-LD parser, every `S3Config` construction
site, and a `FlureeBuilder::s3_force_path_style` setter for embedders
that do not use JSON-LD. Unset leaves the SDK default untouched, so real
AWS is unaffected.

The storage-aws test observes the URL shape directly by presigning a
request — no network — and checks both that `true` puts the bucket in
the path and that unset keeps it as a virtual host. Dropping the SDK
call fails it.
Everything a process needs to run a Raft-replicated nameservice node
already existed — but half of it lived in the fluree-db-server binary,
and the other half was ~180 lines of assembly inside
`FlureeServerBuilder::build`. An embedding process had to re-derive
both from the server's source.

`fluree-db-server/src/raft.rs` moves to
`fluree-db-consensus::raft::integration` verbatim. It never referenced
a server type — the one `crate::` in the file was inside its own tests
— so the move is a relocation, not a redesign. The server re-exports it
at the old path.

The assembly becomes `raft::embedded::EmbeddedRaftNode::attach`: the
queued committer, the ledger-cache watermark hookup, the worker
supervisor, the leader watcher (evictor, liveness monitor, plus
whatever the host adds), and the release task — owned together so they
shut down in the order that keeps the content store consistent:
workers, then leader tasks, then the Raft core, then the release drain.
The background indexer stays a host contribution because this crate
does not depend on fluree-db-indexer; omit it and nothing indexes.

`Fluree::default_index_config` is now public, so an embedder hands the
commit workers the engine's own thresholds instead of constructing
them independently and letting staging and novelty backpressure drift.

The server itself now builds through `EmbeddedRaftNode`, which is how
this is validated: the nine-test cluster suite — failover, follower
forwarding, read-your-write across nodes, liveness demotion, concurrent
writes — runs through exactly the code an embedder calls. A new
`tests/it_embedded_node.rs` stands a node up with no server dependency
at all, mounts the routers at a host-chosen prefix, writes through the
committer, and checks the Raft log advanced and the replicated head and
the engine's cache both agree on the committed t.
`FileStorage` stages a write as `<file>.<pid>.<seq>.tmp` and renames it
into place. That is unique per host only: two nodes of a Raft cluster
sharing a content store over NFS can have the same pid, and each starts
the sequence at zero. Under Raft every node writes the shared store —
whichever node owns a branch stages its commit blob there, the leader
its index artifacts — so the collision is reachable.

It is benign by construction, since the address is the content hash
and both writers hold identical bytes, but the loser's rename fails
and surfaces as a spurious write error on a branch that is in fact
fine.

A 64-bit random token drawn once per process now sits in the name
beside the pid. No node id is plumbed down from whoever knows one: the
token covers the accidental two-servers-one-mount case as well as Raft,
and `fluree-db-core` has no business knowing about cluster identity.
The pid stays because it is what an operator greps for.

The test re-executes the test binary as a child process and asserts
the token component differs; on one host the pids already differ, so
comparing whole names would pass against the old code. Both
reintroducing pid-only names and pinning the token to a constant fail
it.
…ader-only

Both the operations guide and the design doc said the commit worker is
leader-only and that "the leader writes, every node reads" the content
store. The code does the opposite, on purpose: every node runs a worker
supervisor, each branch is assigned to one node by rendezvous hashing
over the worker-eligible voters, and the owning node — routinely a
follower — stages the commit, writes the blob straight to the shared
store, and ferries only the head advance to the leader.

That is the design working as intended, not a discrepancy to correct
toward the docs. The log carries a CID, never the bytes; distributing
the blob-writing half across nodes is what lets that scale. Were
staging leader-only, the leader would be the write bottleneck for the
whole cluster and the narrow log would buy nothing. The docs now say so,
and say that the shared store must be reachable read-write from every
node — the operational consequence an embedder has to plan for.

Also adds the `ledger_exists` integration test the API never had. It
pins, on file storage, that create → commit → exists returns true in
both id forms, and that a malformed id is an Err rather than false —
the trap that turns an existing ledger into a phantom "missing" one
under `.unwrap_or(false)`.

@aaj3f aaj3f left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is well done, @bplatz, and I had Claude validate every problem statement / diagnosis you build against (all verified). All four new tests go red when their fix is reverted, and the deadlock test is deterministic (explicit lock hold, 5/5 on repeat). Stacked on #1679, so no CI ran here; I did the fmt/clippy/nextest work at package scope and it's clean.

The notes are all small: rand landed as an unconditional dep of fluree-db-core even though its only user is behind native and the crate cfg-gates that module out for wasm32 (two-line fix: optional = true + dep:rand under native); the getting-started rust-api.md MinIO example is the builder-API twin of the JSON-LD one you fixed and still needs .s3_force_path_style(true); the vended-credentials scope/grant carry endpoint for MinIO but not path-style, so that consumer path has the same bug one layer up; and the new embedding section of the ops guide could say in one sentence that cluster_admin_router() carries no auth when the host mounts it. Inline for each.

Adherence to repo commitments:

  • Patterns/abstractions: ✔ Deadlock fix extends the file's own stated lock-order rule via one helper; MinIO threads through the existing S3ConfigS3StorageConfig → JSON-LD vocab → FlureeBuilder chain; the Raft assembly is relocated (path rewrites only) and the server consumes it — no parallel construct.
  • Performance (speed first, memory second): ✔ Not on the query path. LedgerHandle clone is an Arc bump on the background indexer's readers; S3 flag is construction-time; staging token is one OnceLock load per staged write. No performance-degradation risk.
  • Testing: ✔ Deadlock regression test (mutation-checked both readers, deterministic), presign-based URL-shape test, JSON-LD parse test, cross-process staging-name test (re-execs the binary; both mutations red), ledger_exists lifecycle test wired via grp_ledger, and an in-process embedded-node boot. The moved integration:: unit tests (8) still pass. The nine-test server cluster suite was not re-run in this pass.
  • Conventions: ✔ Five well-factored commits with full bodies; fmt clean; clippy -D warnings clean across the seven touched crates with all features but vector (whose usearch C++ build fails on my machine — environmental); docs updated in connection-config-jsonld.md, crate-map.md, the ops guide, and the design doc. One rand dep-hygiene nit.

Verified locally at branch HEAD (3f1368f): cargo fmt --all -- --check clean; cargo clippy -p {api,connection,core,storage-aws,consensus,server,nameservice-sync} --all-targets -- -D warnings (all features except vector) clean; targeted cargo nextest run across the five crates → 18/18 including every new test by name; five mutations (reader A, reader B, SDK call dropped, constant token, pid-only names) each turned the corresponding test red.

Approving so you can merge when ready (after #1679, and with a retarget-to-main so CI actually runs), but maybe worth folding the rand gating and the two MinIO doc/vend siblings in first.

Comment thread fluree-db-core/Cargo.toml Outdated
xxhash-rust = { workspace = true }
# Per-process token in `FileStorage` staging-file names. The pid alone
# is unique per host, not across hosts sharing a mount.
rand = { workspace = true }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional (dependency hygiene, wasm32). This is more of a question than a suggestion, since I don't have a wasm32 target on this machine to prove it. rand is added unconditionally, but its only user is file.rs, which we compile under cfg(all(feature = "native", not(target_arch = "wasm32"))) (storage.rs:97). cargo tree -p fluree-db-core --no-default-features now shows rand 0.8.5 → rand_core → getrandom 0.2.17, which wasn't in the no-default tree before, and getrandom 0.2 compile_error!s on wasm32-unknown-unknown unless its js feature is on. Given core, connection, and policy all cfg-gate for wasm32 on purpose, it seems this turns a default-features wasm32 build of core from "compiles" into "doesn't". Nothing in CI builds that target, so it's not a gate — but the fix is two lines and keeps the crate's runtime-agnostic default honest:

Suggested change
rand = { workspace = true }
rand = { workspace = true, optional = true }

with native = ["tokio", "moka", "dashmap", "fs2", "dep:rand"]. Minor and non-blocking — but if you agree it's right, I'd rather see it folded in now than lost in the backlog.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in bf63e54optional = true plus dep:rand under native, exactly your suggestion.

I don't have a wasm32 target here either, so I verified the dependency claim rather than the compile: getrandom v0.2 is now 0 occurrences in cargo tree -p fluree-db-core --no-default-features, and rand v0.8.5 is still there under --features native. The getrandom 0.4.1 that remains in the no-default tree comes only from tempfile as a dev-dependency, so it is not in the library graph — and 0.4 does not carry 0.2's wasm32 compile_error! anyway.

Worth being blunt about severity in the commit body, which I was: this PR introduced it. The staging-token change is what added rand, so a no-default-features wasm32 build of core went from compiling to not compiling as a side effect of a fix about NFS filename collisions — on the one crate whose contract is being runtime-agnostic, and on a target nothing in CI builds. Good catch; "optional, dependency hygiene" undersells it.

SDK still emits virtual-hosted URLs, and a plain MinIO — anything without
wildcard DNS for bucket subdomains in front of it — rejects them. LocalStack
resolves both forms. Leave it unset for real AWS. The builder equivalent is
`FlureeBuilder::s3(bucket, endpoint).s3_force_path_style(true)`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs/getting-started/rust-api.md:142optional (docs). The getting-started S3 example still reads // LocalStack/MinIO: endpoint is required over FlureeBuilder::s3("my-bucket", "http://localhost:4566"), which is the builder-API twin of the JSON-LD MinIO example this PR fixes — against a plain MinIO it still emits virtual-hosted URLs. Since connection-config-jsonld.md now splits the LocalStack and MinIO cases, we may want the same split here (a .s3_force_path_style(true) line for MinIO, or just drop "MinIO" from that comment). Tiny, but it's the example a first-time embedder copies.

Commenting here because docs/getting-started/rust-api.md is not in this diff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in bf63e54. Split it the same way connection-config-jsonld.md now does — LocalStack keeps the plain endpoint example, MinIO gets its own with .s3_force_path_style(true) and a one-line note on why (the SDK emits http://my-bucket.minio:9000/key, which a plain MinIO without wildcard bucket-subdomain DNS rejects).

Agreed on the reasoning: it is the example a first-time embedder copies, and leaving "MinIO" in a comment above something that does not work against MinIO is worse than not mentioning it.

/// being the common case. An endpoint override alone is not enough:
/// the SDK still emits virtual-hosted URLs against it, and a plain
/// `http://minio:9000` rejects those.
pub force_path_style: Option<bool>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fluree-db-nameservice-sync/src/vended_s3.rs:197-202optional (sibling of the MinIO fix). I may be reaching here, but it seems the vended-credentials path is the same bug one layer up: S3VendScope.endpoint is documented as "Non-AWS endpoint override (LocalStack, MinIO), passed through to consumers" (fluree-db-api/src/vended_credentials.rs:29-31), it's copied off the index S3StorageConfig in from_connection_config, the grant carries it (:87), and the consumer builds S3Config { endpoint: grant.endpoint, ..Default::default() } — so a MinIO-backed server that vends credentials hands its sync clients an endpoint they'll address virtual-hosted, and nothing carries force_path_style alongside. One Option<bool> through scope → grant → consumer would close it. I recognize this is separable from the PR's stated scope; if you'd rather keep it out, that's fine, but it's small enough that I'd lean toward folding it in while the knob is fresh.

Commenting here because fluree-db-nameservice-sync/src/vended_s3.rs is not in this diff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in bf63e54. You were not reaching — it is the same bug one layer up, and I'd go further: this PR made it worse rather than neutral. Before, MinIO did not work anywhere, which is at least coherent. After, the server works and silently hands its vended consumers a config that does not, so someone fixes their server, watches it come up, and then debugs why sync fails against the same bucket.

force_path_style now travels with endpoint through scope → grant → consumer.

One thing your note missed, and it would have bitten: there are two VendedS3Grant structs. fluree-db-nameservice-sync keeps its own Deserialize twin of the wire shape because it sits below fluree-db-api. Adding the field only to the consumer you pointed at would have compiled and silently dropped it in transit. Both sides have it now.

Also strengthened the round-trip test rather than just fixing its compile: it pins that the field serializes when set and is omitted when unset, and a new case pins that a grant minted before the field existed still parses. The struct's doc says new fields must be optional and nothing was checking it.

builds the consensus half; `FlureeBuilder::build_client_with_nameservice`
builds the engine against the replicated nameservice;
`fluree_db_consensus::raft::embedded::EmbeddedRaftNode::attach` wires the
two and starts the per-node tasks. The routers carry no path prefix, so the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional (docs, security posture). The new "Embedding without fluree-server" section is really nice — it's the thing an embedder actually needs. One sentence I think it's missing: cluster_admin_router() carries no auth when the host mounts it. The server layers require_admin_token over /cluster/* (fluree-db-server/src/lib.rs:835-838) and the crate's threat model already says embedders "must wrap the router themselves" (raft.rs:66-72), but a reader who only opens the ops guide won't see that. Worth folding in now while the section is being written.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in bf63e54 — a short paragraph at the end of the embedding section saying cluster_admin_router() carries no authentication, that it exposes membership changes (adding and removing voters), that fluree-server layers require_admin_token over /cluster/*, and that an embedding host must wrap it or mount it only on a trusted interface.

You're right that the threat model saying it in raft.rs doesn't help someone who only opens the ops guide — and the whole point of that new section is that it is the thing an embedder reads instead of the source.

Comment thread docs/design/raft-command-queue.md Outdated
| `StagedReceiptMap` | `staged_receipt.rs` | Per-process map carrying typed apply receipts (flake counts, tally, conflict resolution) from worker to transactor on the same node. |

Three of these (`commit_worker::Worker`, `EvictionScheduler`, follower-forward middleware) are gated on leadership: the integration's leader watcher spawns / stops them in response to `current_leader()` changes.
Two of these (`EvictionScheduler`, the background indexer) are gated on leadership: the leader watcher spawns / stops them in response to `current_leader()` changes. `commit_worker::Worker` is deliberately **not**: the worker supervisor runs on every node and owns whichever branches rendezvous-hash to it, so the blob-writing half of a commit is spread across the cluster rather than serialized through the leader. The follower-forward middleware runs everywhere and simply does nothing on the leader.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. "Two of these (EvictionScheduler, the background indexer) are gated on leadership" — the LivenessMonitor is spawned inside the leader watcher too (embedded.rs:186-197), so it's three. So minor, but since this sentence is now the authoritative list, might as well be complete.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in bf63e54 — three, with LivenessMonitor named alongside EvictionScheduler and the background indexer. You're right that it matters more than the size suggests now that the sentence is the authoritative list.

/// cold load of any ledger — and, because the lock is write-fair,
/// every `entries.read()` after that. The manager wedges for the
/// duration of an unrelated write.
async fn ready_handle(&self, ledger_id: &str) -> Option<LedgerHandle> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise. The helper is exactly the shape the file's own invariant asks for, and I checked the rest of the file: with these two routed through ready_handle, no entries guard is held across any await other than the lock acquisition itself. The test is deterministic — it holds lock_for_write() explicitly rather than racing a real transaction — and each single-reader mutation fails it with Elapsed, so both halves of the "reintroducing either alone fails it" claim hold.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — and specifically for checking the rest of the file rather than only the two call sites I changed. "No entries guard is held across any await other than the lock acquisition itself" is the property I wanted to be true and could only assert about the code I touched.

/// observe that directly: presigning builds the real request URL
/// without touching the network.
#[tokio::test]
async fn force_path_style_changes_the_url_shape() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise. Observing the URL shape via presigning instead of a network round-trip is the right way to pin this; dropping the force_path_style call fails it as claimed. And keeping the knob opt-in (Option<bool>, applied only when Some) rather than auto-enabling on endpoint_url is what keeps virtual-hosted-only S3-compatible endpoints working.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciated. The opt-in-vs-auto-enable point is the one I went back and forth on, so having it independently land on the same answer is useful — auto-enabling on endpoint_url would have quietly broken virtual-hosted-only S3-compatible endpoints that work today.

liveness: self.liveness_config.clone(),
extra_leader_tasks: Some(Box::new(leader_tasks)),
};
let node = fluree_db_consensus::raft::embedded::EmbeddedRaftNode::attach(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise. The server building through EmbeddedRaftNode::attach — with only the indexer and BM25 contributed as extra_leader_tasks — is the altitude I was hoping for: one bootstrap, and it_embedded_node.rs boots a real single-voter node in-process with no server import to prove the seam.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. That was the goal — if the server is just another embedder, the seam can't rot, and it_embedded_node.rs proves it without importing the server at all.

Base automatically changed from feature/nameservice-generic-adapter to main August 25, 2026 23:09
bplatz added 2 commits August 25, 2026 19:28
…dentials

Two loose ends from the MinIO and staging-token changes.

`rand` landed as an unconditional dependency of `fluree-db-core` for the
staging token, but its only user is `storage/file.rs`, which the crate
compiles under `native` and not for wasm32. That put `rand -> rand_core ->
getrandom 0.2` into the default-features tree, and getrandom 0.2
`compile_error!`s on wasm32-unknown-unknown without its `js` feature — so a
target that used to build stopped building, on a crate whose whole contract
is being runtime-agnostic. Nothing in CI builds it, so it would have sat
broken silently. Now optional and pulled in by `native`.

`force_path_style` reached `S3Config` but not the vended-credentials path,
which is the same bug one layer up: `S3VendScope` copies `endpoint` off the
index config and the grant carries it to consumers, so a MinIO-backed server
handed its sync clients an endpoint they would address virtual-hosted. The
MinIO fix made that worse rather than better — the server works, its vended
consumers silently do not. The flag now travels with the endpoint through
scope, grant, and consumer. Note the wire shape has two definitions:
`fluree-db-nameservice-sync` keeps its own deserialization twin because it
sits below `fluree-db-api`, and both need the field or it is dropped in
transit.

The grant's round-trip test now pins that the field serializes when set and
is omitted when unset, and a new case pins that a grant minted before the
field existed still parses — the struct documents new fields as optional,
and nothing was checking it.

Docs: the getting-started builder example split LocalStack from MinIO (the
twin of the JSON-LD example already fixed); the leadership-gated task list
in the command-queue design doc says three, not two (`LivenessMonitor` is
spawned in the leader watcher too); and the new embedding section of the
Raft operations guide states that `cluster_admin_router()` carries no
authentication, so a host mounting it must supply its own.
…ck-order-and-minio

Conflict in `fluree-db-server/src/raft.rs`: this branch moves the assembly
into `fluree-db-consensus::raft::integration` and leaves a re-export shim,
while main gained a change inside the file — the server's duplicate
`default_raft_config` and inline election-timeout check collapsed onto
`fluree_raft_core::runtime` (#1678).

Kept the shim, and re-applied the collapse in the file's new home.
`integration.rs` was moved verbatim before that fix landed, so taking this
side alone would have silently reintroduced the duplicated livelock
invariant the move was meant to consolidate.
@bplatz

bplatz commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Two things from this pass that don't belong on any single inline thread:

The merge from main had a trap worth recording. #1679 landed, so this is retargeted to main and origin/main is merged in (5235fa54). The one conflict was fluree-db-server/src/raft.rs: this branch moves the assembly into fluree-db-consensus::raft::integration and leaves a re-export shim, while main had gained a change inside that file — the duplicated default_raft_config and inline election-timeout check collapsed onto fluree_raft_core::runtime (#1678, your review there).

Keeping the shim is right, but on its own it silently reverts that fix: integration.rs was moved verbatim before the collapse landed, so it still carried the duplicate. Re-applied it in the file's new home — integration.rs:257 and :380 now call runtime::check_election_timeout / runtime::default_raft_config, so the livelock invariant still has exactly one definition. Flagging it because a reintroduced duplicate compiles and passes every test, so nothing would have caught it.

Ran the nine-test cluster suite you noted was skipped. 9/9 (1 leaky) on the merged tree. It seemed like the one that most needed it here, since it covers the server booting through EmbeddedRaftNode::attach — this PR's widest change, and now also the carrier of that merge resolution.

Also on the merged tree: clippy -D warnings clean across the six touched crates with all features; core + nameservice-sync + storage-aws + consensus 1254/1254; api grp_ledger + vend + the LocalStack testcontainers grant 155/155; it_embedded_node 1/1. CI is green on all seven checks — the first real run this PR has had, now that the base is main.

@bplatz
bplatz merged commit 10cb0a9 into main Aug 26, 2026
14 checks passed
@bplatz
bplatz deleted the fix/ledger-manager-lock-order-and-minio branch August 26, 2026 01:08
@bplatz bplatz mentioned this pull request Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants