experimental/air: content-address the plain_tar upload - #6578
Merged
Merged
Conversation
Collaborator
Integration test reportCommit: cff4208
Top 3 slowest tests (at least 2 minutes):
|
git_archive snapshots are already content-addressed: a repeat submission at the same commit reuses the uploaded tarball and skips packaging + upload. plain_tar (dirty working tree) used a timestamped name, so it re-packaged and re-uploaded the full tarball on every submission, even when nothing changed. Name the plain_tar tarball by a working-tree fingerprint (sha256 over each file's path, size and mtime) and run the same snapshotExists skip for both modes. An unchanged resubmit now reuses the remote object and moves no bytes. The listing is captured once and threaded into packaging, so the tree is walked only once. The fingerprint is size+mtime, not content, matching DABs file-sync. Verified on df1: a second submission of an unchanged tree logs "snapshot upload skipped; reusing ..." and returns the identical remote path. Co-authored-by: Isaac <no-reply@databricks.com>
ben-hansen-db
force-pushed
the
air-plain-tar-content-addressed
branch
from
September 9, 2026 16:03
9f7d8be to
b9aa788
Compare
- Strengthen the dedup test: count import-file calls and assert the second
(unchanged) submit adds zero, instead of asserting a path-keyed set has one
entry. The set couldn't distinguish a skipped submit from a re-upload to the
same content-addressed name; the counter can (verified it fails when the skip
is disabled).
- Fold snapshotPackagingVersion into computePlainTarKey so a packaging-logic
bump invalidates plain_tar keys too, not just plainTarKeyVersion.
- Fix stale comments now that plain_tar is content-addressed: modePlainTar
("not cacheable") and snapshotExists ("git_archive" only).
Co-authored-by: Isaac <no-reply@databricks.com>
ben-hansen-db
marked this pull request as ready for review
September 15, 2026 16:15
vinchenzo-db
approved these changes
Sep 15, 2026
…-addressed # Conflicts: # experimental/air/cmd/runsubmit_test.go # experimental/air/cmd/snapshot_dabs.go
ben-hansen-db
marked this pull request as draft
September 15, 2026 17:04
ben-hansen-db
marked this pull request as ready for review
September 15, 2026 17:28
janniklasrose
pushed a commit
that referenced
this pull request
Sep 15, 2026
## Summary Makes the `plain_tar` upload content-addressed so an unchanged working tree skips packaging **and** upload. Today `git_archive` names its tarball by `(commit, include_paths)` and, if that object is already uploaded, reuses it and moves no bytes. `plain_tar` (dirty working tree / no ref — the normal iterate-and-resubmit loop) used a **timestamped** name, so it re-packaged and re-uploaded the full tarball on *every* submission, even when nothing changed. This change: - **`snapshotFiles`** records each path's size, mtime, mode/type, and symlink target. Size, mtime, and mode/type come from the existing `Lstat`; only symlinks require an additional `Readlink`. - **`computePlainTarKey`** hashes the sorted `path+size+mtime+mode+symlink_target` set into a versioned working-tree fingerprint. - **`snapshotTarName`** names `plain_tar` `<dir>_<fingerprint>.tar.gz` (was `<dir>_<timestamp>.tar.gz`) and returns the file listing so packaging reuses it — the tree is walked once, not twice. - **`uploadSnapshotViaDABs`** runs the existing `snapshotExists` skip for **both** modes: if the object already exists, it reuses the remote path and uploads nothing. ## Validation - `go test ./experimental/air/cmd` passes. Tests cover remote deduplication and verify that path, size, mtime, mode/type, and symlink-target changes independently alter the cache key. - `./task lint-q` passes with zero issues. - **End-to-end on df1.** Submitting an unchanged universe tree twice: the second run logged `snapshot upload skipped; reusing …<fingerprint>.tar.gz`, returned the identical remote `code_source_path`, and skipped packaging + upload. Both runs launched (1×A10): | code source | first submit (package + upload) | second submit (unchanged → skipped) | |---|---|---| | `research` (~27 MB) | 5.7 s | 4.9 s | | `research` + `js` + `spark` (~97 MB) | 37.6 s | 4.8 s | These measurements preceded the parallel gzip improvement in #6571, so the first-submit numbers are historical; the unchanged-tree skip remains the behavior demonstrated here. ## Notes / trade-offs - Regular-file content detection is size+mtime, not content — the same trade-off DABs file sync makes — so an edit preserving both is not detected. Mode/type and symlink-target changes are detected. - Remote objects accumulate (one per distinct tree state), same as `git_archive`; no eviction is added here. --- This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 0604fb0
231 interesting tests: 221 FAIL, 7 KNOWN, 2 flaky, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
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.
Summary
Makes the
plain_tarupload content-addressed so an unchanged working tree skips packaging and upload.Today
git_archivenames its tarball by(commit, include_paths)and, if that object is already uploaded, reuses it and moves no bytes.plain_tar(dirty working tree / no ref — the normal iterate-and-resubmit loop) used a timestamped name, so it re-packaged and re-uploaded the full tarball on every submission, even when nothing changed.This change:
snapshotFilesrecords each path's size, mtime, mode/type, and symlink target. Size, mtime, and mode/type come from the existingLstat; only symlinks require an additionalReadlink.computePlainTarKeyhashes the sortedpath+size+mtime+mode+symlink_targetset into a versioned working-tree fingerprint.snapshotTarNamenamesplain_tar<dir>_<fingerprint>.tar.gz(was<dir>_<timestamp>.tar.gz) and returns the file listing so packaging reuses it — the tree is walked once, not twice.uploadSnapshotViaDABsruns the existingsnapshotExistsskip for both modes: if the object already exists, it reuses the remote path and uploads nothing.Validation
go test ./experimental/air/cmdpasses. Tests cover remote deduplication and verify that path, size, mtime, mode/type, and symlink-target changes independently alter the cache key../task lint-qpasses with zero issues.End-to-end on df1. Submitting an unchanged universe tree twice: the second run logged
snapshot upload skipped; reusing …<fingerprint>.tar.gz, returned the identical remotecode_source_path, and skipped packaging + upload. Both runs launched (1×A10):research(~27 MB)research+js+spark(~97 MB)These measurements preceded the parallel gzip improvement in experimental/air: parallel gzip for the plain_tar snapshot packer #6571, so the first-submit numbers are historical; the unchanged-tree skip remains the behavior demonstrated here.
Notes / trade-offs
git_archive; no eviction is added here.This pull request and its description were written by Isaac.