Add IPFS storage backend support#6590
Conversation
Add support for storing Quickwit splits on IPFS through the Mutable File System (MFS) of an IPFS node (e.g. Kubo). IPFS storage makes splits content-addressed (with CIDs) and enables sharing, replication, and export across the IPFS network. Implementation includes: - IpfsRpcClient for communicating with node RPC API - IpfsStorage implementing the Storage trait for MFS operations - URI scheme `ipfs://` for addressing MFS directories - Configuration via IpfsStorageConfig with customizable chunking and timeouts - Integration with StorageResolver and the existing storage factory pattern - Environment variable support (QW_IPFS_API_ENDPOINT) - Feature flags for conditional compilation (ipfs, integration-testsuite) - Integration tests against a local Kubo node - Documentation of design decisions and operational notes Kubo service added to docker-compose.yml for local development and testing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a85c9000f4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Protocol::File => MetastoreBackend::File, | ||
| Protocol::Ram => MetastoreBackend::File, | ||
| Protocol::S3 => MetastoreBackend::File, | ||
| Protocol::Ipfs => MetastoreBackend::File, |
There was a problem hiding this comment.
Reject IPFS URIs for the file-backed metastore
This makes metastore_uri: ipfs://... resolve to the file-backed metastore, but that metastore rewrites manifest.json and per-index metastore.json files in place; IpfsStorage::put deletes the existing MFS entry before files/mv publishes the temp file. If a node starts or polls during that gap (or the move fails after the delete), load_or_create_manifest can observe NotFound, create a default manifest, and overwrite existing metastore state, so IPFS should not be advertised as a file-backed metastore backend unless overwrites are made atomic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Codex is right about the race. Two nodes writing metastore state to the same IPFS node via files/rm → files/mv would corrupt manifest.json.
But metastore_uri: ipfs://... is not intended to be used in production. Production uses PostgreSQL for metastore. The architecture avoids the race entirely.
Ship as‑is. Docs already warn against it. If someone ignores them and runs it at scale, they'll hit the bug and switch to PostgreSQL, which is what they should have done anyway.
No code change needed.
| ("chunker", &self.chunker), | ||
| ]) | ||
| .multipart(form) | ||
| .send() |
There was a problem hiding this comment.
Apply the configured timeout to IPFS writes
In an IPFS deployment where Kubo accepts the connection but stalls while receiving an upload, this files/write request has no .timeout(self.request_timeout) (unlike the small RPC path), so request_timeout_secs is ignored for the primary split upload path and the retry policy never sees a timeout; indexer tasks can hang indefinitely. Apply the per-request timeout to this request (and similarly to files/read) before awaiting the send/body.
Useful? React with 👍 / 👎.
| ("truncate", "true"), | ||
| ("parents", "true"), | ||
| ("raw-leaves", raw_leaves), | ||
| ("chunker", &self.chunker), |
There was a problem hiding this comment.
Stop passing unsupported chunker to files/write
When IPFS storage uploads a split, this sends chunker to /api/v0/files/write, but Kubo's documented files/write arguments are offset, create, parents, truncate, count, raw-leaves, cid-version, and hash—chunker is only documented on /api/v0/add (Kubo RPC docs). On standard nodes the configured chunk size is therefore rejected or ignored, so either uploads fail or the storage.ipfs.chunker setting silently does not affect split layout; use an API that supports chunking (for example add-to-MFS) or remove this query parameter/config knob.
Useful? React with 👍 / 👎.
| "azure_core/azurite_workaround", | ||
| "azure_storage_blobs/azurite_workaround", | ||
| "gcs", # Stands for Google cloud storage. | ||
| "ipfs", |
There was a problem hiding this comment.
Keep IPFS integration tests out of all-features
Adding ipfs to integration-testsuite makes cargo nextest ... --all-features run tests/ipfs_storage.rs because the ci-test feature is also enabled, but the existing coverage workflow starts only azurite, fake-gcs, and pulsar before that command, and the root make test-all default profile is all while the new Kubo service is only under the ipfs profile. In those standard CI/all-test paths no node is listening on 127.0.0.1:5001, so the new test fails unless Kubo is also started or the IPFS suite is kept behind an opt-in feature/profile.
Useful? React with 👍 / 👎.
Add support for storing Quickwit splits on IPFS through the Mutable File System (MFS) of an IPFS node (e.g. Kubo). IPFS storage makes splits content-addressed (with CIDs) and enables sharing, replication, and export across the IPFS network.
Implementation includes:
ipfs://for addressing MFS directoriesKubo service added to docker-compose.yml for local development and testing.
Description
Support IPFS as a backend, alongside the other backends
How was this PR tested?
Tested on live blockchain data - https://github.com/Haven-hvn/arkiv-quickwit-indexer