Write ingest output to multiple buckets - #1248
Open
macpie wants to merge 1 commit into
Open
Conversation
Every file ingest writes to `output_bucket` can now be copied, byte for
byte and under the same key, to any number of additional buckets. The
motivating case is mirroring an S3 primary to Cloudflare R2, so the
buckets do not share a provider.
output_bucket = "ingest-bucket"
[additional_output_buckets.r2]
bucket = "ingest-bucket-mirror"
endpoint = "https://<account_id>.r2.cloudflarestorage.com"
region = "auto"
access_key_id = "..."
secret_access_key = "..."
Defaults to empty, so existing deployments are unaffected.
A map keyed by name rather than a list: `config` builds nested maps from
`INGEST__`-prefixed environment variables but cannot build sequences, so
a list would strand every mirror's credentials in the settings file. The
name also identifies the bucket in log lines and config errors.
Each entry is self-contained and inherits nothing from `[file_store]`.
Lending an R2 bucket the S3 account's region or key pair only builds a
client that authenticates against neither. For the same reason ingest
refuses to start on an entry with only one half of a credential pair:
the S3 client silently ignores both and falls back to the ambient AWS
credential chain, which cannot succeed against R2, so a typo would fail
every upload at runtime instead of at startup.
A local file is deleted only once every bucket has it, so a failing
bucket never costs the others their copy. A file left behind is retried
on the next restart, when the sink rescans its cache directory. Nothing
retries it in-process, so a mirror that stays broken accumulates files
on disk.
Adds `file_store_upload{bucket,status}`, one increment per file per
bucket at its terminal outcome. Both series are seeded to zero at
startup so an alert can distinguish "no failures" from "not reporting".
Without it a bucket that quietly stops accepting files is invisible:
the uploader keeps the local copy, logs, and the service looks healthy.
Two fixes fell out of testing this:
- A `roll_time` shorter than 60s was silently ignored. Time-based
rolling only happens on a rollover timer whose period was a flat 60s
constant; the write path checks size alone. The interval is now
min(roll_time, 60s), which leaves every deployment unchanged (3 min
default, 15 for ingest) and lets the `cfg(test)` override go.
- `AwsLocal::delete_bucket` could not delete an empty bucket. It always
sent DeleteObjects, which S3 rejects with no keys (MalformedXML), so a
bucket created but never written to could not be torn down.
Tests are end to end against the RustFS in docker-compose, on buckets
AwsLocal creates and deletes per run. ingest/tests/multi_output_bucket.rs
drives the real grpc_server from a real settings file through the real
sinks and uploader, and asserts both buckets hold the same key with
identical bytes that decode back to the submitted report.
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.
Every file ingest writes to
output_bucketcan now be copied, byte for byte and under the same key, to any number of additional buckets. The motivating case is mirroring an S3 primary to Cloudflare R2, so the two buckets do not share a provider or a key pair.Defaults to empty, so existing deployments are unaffected.
FileUpload::newandfrom_bucket_clientkeep their signatures, soprice,mobile-verifierandmobile-packet-verifierare untouched.Why a map and not a list
configbuilds nested maps fromINGEST__-prefixed environment variables but cannot build sequences. AVecwould have stranded every mirror's credentials in the settings file with no way to inject them as env vars — verified iningest/tests/settings_env.rs, which configures a whole mirror, secret included, from the environment:(Note the separator is
__after the prefix too —INGEST_MODEis silently ignored.) The map key also names the bucket in log lines and config errors.Why entries inherit nothing from
[file_store]Each entry is self-contained. Lending an R2 bucket the S3 account's region or key pair only builds a client that authenticates against neither.
For the same reason ingest now refuses to start on an entry carrying only one half of a credential pair: the S3 client installs static credentials only when it has both, and otherwise silently falls back to the ambient AWS credential chain — which cannot succeed against R2. A typo in one of the two key names would have passed startup and failed every upload at runtime.
No changes were needed in
new_clientfor R2: it already appliesforce_path_stylewhen an endpoint is set, and already forcesrequest_checksum_calculation(WhenRequired), which is what keeps R2 from choking onaws-chunkedtrailing checksums. Clients are cached on the full credential tuple, so the two stay distinct.What happens when one bucket fails
Buckets are attempted concurrently, each with its own retry loop (~60s). A local file is deleted only once every bucket has it, so a failing bucket never costs the others their copy. A file left behind is retried on the next restart, when
FileSink::initrescans the cache directory.Nothing retries it in-process, so a mirror that stays broken accumulates files on the ingest host's disk. That is the deliberate tradeoff — never lose a copy — but worth knowing before enabling a mirror. A periodic cache rescan would make it self-healing; happy to add that if reviewers want it now.
Observability
Adds
file_store_upload{bucket,status}— one increment per file per bucket at its terminal outcome, sostatus="error"counts files that bucket actually dropped:Both series are seeded to zero per bucket at startup so an alert can tell "no failures" from "not reporting at all". Without the metric, a bucket that quietly stops accepting files is invisible: the uploader keeps the local copy, logs an error, and the service goes on looking healthy.
Two fixes that fell out of testing
A
roll_timeshorter than 60s was silently ignored. Time-based rolling happens only on a rollover timer whose period was a flatSINK_CHECK_MILLIS = 60_000; the write path checks size alone (will_fit). So any configured roll time under a minute was rounded up to one — which is whyfile_storecarried a#[cfg(test)]50ms override, and why the new ingest integration test initially hung. The interval is nowmin(roll_time, 60s), floored at 1ms sinceintervalpanics on zero.Production behaviour is unchanged: every deployment configures ≥ 60s (3 min default, 15 for ingest), so
minalways picks 60s. Thecfg(test)override is gone andfile_store's own sink tests now pass against the real constant.mobile_packet_verifier's integration tests setroll_time(100ms)with a comment saying it "ensures the file is committed promptly" — that only became true with this change; all 78 still pass.AwsLocal::delete_bucketcould not delete an empty bucket. It always sentDeleteObjects, which S3 rejects with no keys (MalformedXML, HTTP 400), so a bucket created but never written to could not be torn down at all.Tests
All end to end against the RustFS in
docker-compose.yml, on bucketsAwsLocalcreates and deletes per run — noinit.shchange, no cross-run collisions.ingest/tests/multi_output_bucket.rsdrives the realgrpc_serverfrom a real settings file through the real sinks and uploader: gRPC submission → rolling sink → both buckets. Asserts the same key, byte-identical content, and that those bytes decode back to the submitted report. Unlikemobile_ingest.rs, nothing is stubbed.file_store/tests/file_upload.rscovers the fan-out, the keep-on-partial-failure behaviour, a mirror under an independent credential pair, and the metric's label shape.ingest/src/settings.rsunit tests cover the config shape, the credential-pair guard, and that a settings dump omits mirror credentials —main.rslogs the whole struct as JSON at startup, and theskip_serializingon the credential pair now sits behind aserde(flatten).Each assertion was mutation-checked (dropping the mirrors, removing the metric increment, removing the seeding) to confirm it fails when the behaviour breaks.
CI
Verified against the pinned 1.94 toolchain, not the newer one on my machine —
cargo fmt --all --checkandcargo clippy --all-targets --locked -- -Dclippy::all -D warningsare both clean workspace-wide. Full suites pass forfile-store,ingest,mobile-packet-verifier,mobile-verifier,priceandfile-store-oracles.One unrelated flake to be aware of:
file_info_poller::poller_without_idle_timeout_does_not_exithas a 100ms timeout on an S3 round-trip and fails intermittently on a loaded worker. It predates this branch.metrics-utilis added as a dev-dependency only (already in the lockfile transitively); enabling itsdebuggingfeature pulls a few new crates intoCargo.lock, so the first CI run fetches them rather than getting them pre-warmed from the base image.