feat(storage): cut directory-upload round trips and widen small-object fan-out - #3638
Draft
vaibhavatlan wants to merge 1 commit into
Draft
vaibhavatlan wants to merge 1 commit into
vaibhavatlan wants to merge 1 commit into
Conversation
…t fan-out A directory hand-off over a high-latency store is bound by requests, not bytes. Every file in a directory upload cost four dependent round trips — a sidecar HEAD for the skip check, the PUT, a readback HEAD, the sidecar PUT — drained four at a time. A production agent run that staged ~19k small parquet files spent 2h53m in App.upload at ~0.5s per request through the tenant blobstorage gateway; two earlier runs of the same connector timed out in it. The SDK's own work was ~10 ms per file (measured against a local store); the rest was round trips (FND-1339). transfer.upload, directory mode: - One listing of the target prefix before the transfer answers the skip_if_exists check for every file (read_expected_digest already accepts sidecar_present; it is now fed from the listing instead of a HEAD per file). - One listing after the transfer is the readback for every object at once — the same two checks (present, and the size that was sent) and the same error classes as the per-object HEAD in _finalize_upload_integrity. - Sidecars are written only after that readback passes, preserving the ordering guarantee the per-object protocol makes. - Fan-out is sized per file: objects at or below STORAGE_SMALL_OBJECT_BYTES (4 MiB) run MAX_CONCURRENT_SMALL_TRANSFERS (32) wide; larger objects keep MAX_CONCURRENT_STORAGE_TRANSFERS (4) because each holds part-size x part-concurrency of buffer. An explicit max_concurrency is one hard cap. - Net: 4 -> 2 requests per object plus a request per thousand keys, at up to 8x the fan-out for the round-trip-bound tier. upload_file gains defer_remote_verify for callers that take over the readback; the single-file path is unchanged. FileReference directory persist uses the same tiered fan-out. App.upload: the warn-only transformed-asset validation runs alongside the transfer instead of ahead of it, is skipped on retry attempts, and its bound is capped at half the activity's start-to-close timeout — the two used to share a 600s default, so a scan over millions of records could spend the whole activity budget before the first PUT. New env knobs: ATLAN_MAX_CONCURRENT_SMALL_TRANSFERS, ATLAN_STORAGE_SMALL_OBJECT_BYTES (0 disables the tier).
vaibhavatlan
requested review from
Aryamanz29,
OnkarVO7,
atlan-ci and
cmgrote
as code owners
September 2, 2026 15:50
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 81.1%) Detailed Coverage ReportThis message was truncated. Download full message |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailspackages/conformance/uv.lockuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailspackages/conformance/uv.lockuv.lock |
Contributor
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
5 tasks
vaibhavatlan
marked this pull request as draft
September 2, 2026 17:23
This branch has not been deployed
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
1 participant
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.
Changelog
skip_if_existscheck for every file (read_expected_digestalready tooksidecar_present; it is now fed from the listing). After the transfer, one listing is the readback for every object at once: same two checks as the per-object HEAD in_finalize_upload_integrity(present, and the size that was sent), same error classes. Sidecars are written only after that readback passes, so the "never advertise an object this upload rejected" ordering is preserved.transfer.uploadandFileReferencedirectory persist). Objects at or belowATLAN_STORAGE_SMALL_OBJECT_BYTES(4 MiB) runATLAN_MAX_CONCURRENT_SMALL_TRANSFERS(32) wide; larger objects keepATLAN_MAX_CONCURRENT_STORAGE_TRANSFERS(4) because each holds part-size × part-concurrency of buffer. An explicitmax_concurrencystays one hard cap across everything in flight.0disables the tier.upload_file(defer_remote_verify=...): local-truncation check on, readback HEAD off, for callers that take over the readback. Single-file uploads are unchanged.App.upload: the warn-only transformed-asset validation runs alongside the transfer instead of ahead of it, is skipped on retry attempts (deterministic over the same tree; attempt 1 emitted its outcome), and its bound is capped at half the activity's start-to-close timeout. The two used to share a 600s default, so a scan over millions of records could spend the whole activity budget before the first PUT.configuration.md,storage.md(new Directory uploads: round trips, not bytes section),file-reference.md.Why
A directory hand-off over a high-latency store is bound by requests, not bytes. Every file cost four dependent round trips (sidecar HEAD, PUT, readback HEAD, sidecar PUT) drained four at a time. A production agent-mode run that staged ~19k small parquet files spent 2h53m in
App.uploadat ~0.5 s per request through the tenant blobstorage gateway; two earlier runs of the same connector died in it after 3 × 600s. The SDK's own work was ~10 ms per file (measured against a local store); the rest was round trips.Measured against an S3-compatible stub that logs requests and injects 100 ms each (200 files):
skip_if_exists, verify, sidecars)Net for the SAP-shaped tree: 4 → 2 requests per object plus one request per thousand keys, at up to 8× the fan-out for the round-trip-bound tier. Object count is still what scales the cost; the writer-side follow-up (roll by size, fix the 5,000-row consolidation slices) is a separate PR.
Additional context (e.g. screenshots, logs, links)
ParquetFileWriterconsolidation slicing atchunk_size;RollingFileWriterminimum-bytes guard on the time rollover; a per-prefix integrity manifest to drop the sidecar PUT as well (touches the reader side, needs its own design note).ATLAN_STORAGE_VERIFY_TRANSFERS=falsestill means no readback at all; with it on, directory mode's readback is the listing. Listing consistency assumptions match the HEAD's (read-after-write).Checklist
tests/unit/storage/test_transfer.py: request log + fan-out tiers;test_integrity.py: deferred readback;test_reference.py: tiered persist;tests/unit/app/test_base.py+test_upload_asset_validation.py: validation alongside the transfer, cancel on failure, retry skip, budget cap)Copyleft License Compliance