osm slab: hydrate from S3 onto the persistent volume, checksum-pinned - #112
Merged
Conversation
Both were checked off in Phase 5's own list without closing their originals higher in the file, so a grep for open items reported the hip-cell comparison and the quadratic render as pending when both had shipped and been measured. Same doc-drift the tesseract-rs D3/D4 case burned time on: the summary line went stale the moment it was written because nothing re-checked it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f0fa7501-5edb-4d9e-a30c-9802b7b5c7eb) |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe plan records completed cockpit rendering work. The server now hydrates OSM slab artifacts from S3 into a volume cache, verifies SHA-256 checksums, and performs hydration before binding. ChangesCockpit completion records
OSM slab hydration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ServerStartup
participant ensure_slab_local
participant S3
participant VolumeCache
ServerStartup->>ensure_slab_local: hydrate before server bind
ensure_slab_local->>S3: fetch checksum manifest
ensure_slab_local->>VolumeCache: validate cached slab files
ensure_slab_local->>S3: download missing or invalid files
S3-->>ensure_slab_local: streamed artifact data
ensure_slab_local->>VolumeCache: atomically store verified files
ensure_slab_local-->>ServerStartup: return local slab path or None
Possibly related PRs
Suggested reviewers: Poem
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closes the one item that blocked the POC from running anywhere but this
sandbox. Operator: "railway has exactly the same S3 variables like you" — which
retired the blocker, because the volume is a PATH and the part that needed
proving (do these credentials reach that bucket, does a 1.29 GiB round trip
work) is verifiable right here. OSM_SLAB_CACHE_DIR substitutes for the mount.
TOPOLOGY, per the operator's framing ("S3 -> lancedb -> volume01", "persist the
lance file to volume01", "it's just a convenience to persist lance across
rebuilds"): S3 is the source of truth, the volume is a CACHE that survives
container rebuilds. Deleting it costs a re-download and nothing else.
THREE ASSUMPTIONS I HAD WRITTEN DOWN, ALL WRONG. The earlier sketch claimed
this needed ~5 new deps (reqwest+rustls, sha2, hex, chrono, tokio sync),
hand-rolled SigV4, and a sync->async conversion of open_slab(). Checked instead
of repeated:
claimed measured
needs a new HTTP stack object_store 0.13.2 is ALREADY in the graph
via datafusion/lance; no new transitive tree
needs hand-rolled SigV4 AmazonS3Builder::from_env() reads it all, and
`aws_endpoint_url` is an accepted alias for the
endpoint key (builder.rs:498), so a non-AWS
endpoint needs no special casing
open_slab() must become async main() is ALREADY async — hydrating there,
before the listener binds, leaves the read path
untouched and stops the first request paying
for the download
FORMAT-AGNOSTIC BY CONSTRUCTION. openstreetmap-website-rs states in its own
Cargo.toml header: "no lance, no datafusion: the ABI row is a byte layout, not
an engine" — and lance_abi was REMOVED there (a27b06a) for doing a full-table
materialising read. So this layer moves named objects from a prefix onto the
volume and verifies them; whether the artifact travels as .soa or as a Lance
dataset is a separate decision it does not prejudge. (object_store exposes
get_range(location, Range<u64>) — the range read that removal asked for, if the
Lance path is taken later.)
CHECKSUM PINNING, ON THE CACHE HIT TOO. SHA256SUMS is fetched from the same
prefix; a missing entry is a refusal, not a warning. Verifying on a HIT — not
only after download — guards the half-written file left by a container killed
mid-download, exactly the case a "we already have it" check waves through.
Downloads land on .part and are renamed only after the hash matches.
VERIFIED, all three paths, against the live bucket:
cold empty volume -> downloaded 1,292,826,624 + 58,763,338 B, verified,
mmap'd; serves 14/8802/5373 as 15,016 of 15,016 — identical to the
local-file numbers
warm cache hit, both artifacts checksum-verified in ~0.8s, no download
dirty 16 B corrupted mid-file -> got=7247b2bb… want=d12bc8a1… detected,
re-fetched, verified, no .part left behind
Both halves of the guard are proven on real data: silent on a clean cache, loud
on a corrupt one.
FOUND WHILE DOING IT — the server's own tracing has never been emitted. The
filter said `cockpit_server=info`, but the binary TARGET is `q2-cockpit`, so
module_path!() starts with `q2_cockpit`. Every tracing::* call in this binary
was silently discarded. Noticed because the hydrate ran correctly and logged
nothing; on a deploy that would have been 503s with no explanation at all. Both
names are now listed so a rename of either cannot silence it again, and
RUST_LOG wins when set — the operational control a deploy needs.
Bucket conventions followed, not invented: the bucket already had 50+ MedCare-rs
objects shaped <repo>/bakes/<version>/<artifact> + SHA256SUMS. q2 uploads to
q2/bakes/berlin-v1/ and touches nothing under MedCare-rs/.
90 passed, 0 failed, 3 ignored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
AdaWorldAPI
marked this pull request as ready for review
August 11, 2026 08:55
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8de54985-c770-41fa-9bee-23560b548594) |
This was referenced Aug 11, 2026
This was referenced Aug 11, 2026
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
2 participants
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.
Closes the one item that blocked the POC from running anywhere but this sandbox, plus the plan-hygiene commit it was branched on.
"railway has exactly the same S3 variables like you" retired the blocker. Phase 4 was recorded as "verifiable only on deploy" because
/volume01doesn't exist here — but the volume is a path, and the part that needed proving (do these credentials reach that bucket, does a 1.29 GiB round trip work) is verifiable right now.OSM_SLAB_CACHE_DIRsubstitutes for the mount, so everything except the literal mount point is exercised locally.Topology
Per the framing — "S3 → lancedb → volume01", "persist the lance file to volume01", "it's just a convenience to persist lance across rebuilds" — S3 is the source of truth, the volume is a cache that survives container rebuilds. Deleting it costs a re-download and nothing else.
Three assumptions I had written down, all wrong
The earlier sketch claimed ~5 new deps, hand-rolled SigV4, and a sync→async conversion of
open_slab(). Checked instead of repeated:reqwest+ a new HTTP stackobject_store0.13.2 is already in the graph viadatafusion/lance— adding it direct pulls no new transitive treeAmazonS3Builder::from_env()reads it all, andaws_endpoint_urlis an accepted alias for the endpoint key (builder.rs:498), so a non-AWS endpoint needs no special casingopen_slab()must become asyncmain()is already async — hydrating there leaves the read path untouched and stops the first request paying for the downloadFormat-agnostic by construction
openstreetmap-website-rsstates in its ownCargo.tomlheader: "no lance, no datafusion: the ABI row is a byte layout, not an engine" — andlance_abiwas removed there (a27b06a) for doing a full-table materialising read. So this layer moves named objects onto the volume and verifies them; whether the artifact travels as.soaor as a Lance dataset is a separate decision it does not prejudge.Worth carrying from that removal, since it names the right shape if the Lance path is taken later: "this crate already has fragment.rs (64k-row fragments, so a tile prefix lands in a fragment) and
RowSlab::tile_range. The module used neither."object_storeexposesget_range(location, Range<u64>)— exactly that range read.Checksum pinning, on the cache hit too
SHA256SUMSis fetched from the same prefix; a missing entry is a refusal, not a warning. Verifying on a hit — not only after download — guards the half-written file left by a container killed mid-download, which is precisely the case a "we already have it" check waves through. Downloads land on.partand are renamed only after the hash matches.Verified — all three paths, against the live bucket
14/8802/5373as 15,016 of 15,016 — identical to the local-file numbersgot=7247b2bb… want=d12bc8a1…detected, re-fetched, verified, no.partleft behindBoth halves of the guard are proven on real data: silent on a clean cache, loud on a corrupt one.
⚠ Found while doing it — the server's tracing has never been emitted
The filter said
cockpit_server=info, but the binary target isq2-cockpit, somodule_path!()starts withq2_cockpit. Everytracing::*call in this binary was silently discarded. Noticed because the hydrate ran correctly and logged nothing — on a deploy that would have been 503s with no explanation at all. Both names are now listed so a rename of either cannot silence it again, andRUST_LOGwins when set, which is the operational control a deploy needs.Bucket conventions — followed, not invented
The bucket already held 50+ MedCare-rs objects shaped
<repo>/bakes/<version>/<artifact>+SHA256SUMS, with Lance datasets as.lance.tar. q2 uploads toq2/bakes/berlin-v1/and touches nothing underMedCare-rs/.q2/bakes/berlin-v1/berlin.soaq2/bakes/berlin-v1/berlin.booksq2/bakes/berlin-v1/SHA256SUMSAlso in this PR
The plan-hygiene commit this branched on: two follow-ups were checked off inside Phase 5's own list without closing their originals higher in the file, so a grep for open items reported the hip-cell comparison and the quadratic render as pending when both had shipped and been measured.
Verification
cargo test --bin q2-cockpit— 90 passed, 0 failed, 3 ignored.Still unverified: the literal
/volume01mount. Everything else in the chain is exercised above; first boot on Railway is what tests the mount point itself.🤖 Generated with Claude Code
https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
Summary by CodeRabbit
New Features
Bug Fixes