List part files lazily during upload instead of the whole table (#1550) - #1551
Merged
Merged
Conversation
splitFilesByName walked every part of a table before the first upload goroutine started and kept the relative path of every file (sharing the backing array of the full path, so the shadow prefix was retained too) until the last part was uploaded. On tables with 1000+ columns and thousands of Wide parts that is millions of strings: 4 GB live heap and 10 GB RSS observed on a 2.8.0 server. With upload_by_part=true the part directory is now walked inside the per-part upload goroutine, after the resumable-state check, so only upload_concurrency part file lists are alive at a time. splitFilesBySize still lists everything (it cuts by max_file_size) but clones the relative path so the prefix is not retained. Fix #1550 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Coverage Report for CI Build 34815986991Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.03%) to 67.04%Details
Uncovered Changes
Coverage Regressions41 previously-covered lines in 9 files lost coverage.
Coverage Stats
💛 - Coveralls |
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.
Fix #1550
Where the memory went
Production case:
clickhouse-backup server2.8.0 at 9.4-10.8 GB RSS duringcreate_remote. Heap profile (api.enable_pprof: true,GET /debug/pprof/heap):The database: two tables with 1115 columns,
PARTITION BY Date, ~3400 partitions -> ~3900 activeWideparts of ~12 MB each (min_bytes_for_wide_part= 10 MB). ~2500 files per part (<column>.bin,.cmrk2,.sparse.idx.bin).find | wc -lon the backup shadow dir: 9.8M and 10.5M files in the two tables, 21.7M files in the whole backup. Both tables were being uploaded at the same time (upload_concurrency: 2), so ~20 million relative paths were resident.Two things made that expensive:
splitFilesByNamewalked every part of the table before the first upload goroutine started, and the resulting[]metadata.SplitPartFilesstayed referenced untiluploadTableDatareturned. Memory was O(files in the table), while the upload only needs the files of theupload_concurrencyparts currently being archived.relativePath := strings.TrimPrefix(filePath, basePath)is a substring of the full path allocated byfilepath.Walk, so every stored relative path also pinned the ~110 byte/var/lib/clickhouse/backup/<backup>/shadow/<db>/<table>/<disk>/prefix. Measured on 1M synthetic paths of that shape:20M × 178 B ≈ 3.4 GB, matching the 3.5 GB of
MakeNoZeroin the profile.How we save it
upload_by_part: true(default):splitFilesByNamenow returns only the part names. The directory walk moved intowalkPartFiles, called inside the per-part upload goroutine right beforeUploadCompressedStream/UploadPath, after theresumableState.IsAlreadyProcessedcheck. Consequences:upload_concurrencypart file lists are alive per table, instead of the whole table: for the case above ~2 × 2500 paths instead of ~10M per table--resume) skip the directory walk entirelyupload_by_part: false: the list still has to be built up front to cut archives bymax_file_size, but each relative path isstrings.Cloned, so the basePath prefix is no longer retained (~3× less per file).skipProjectionsfiltering (Add option skip_projections with table pattern to allow make backup without projection #861), sameRequiredparts handling, same warning on walk errors.TestWalkPartFilesMemory(unit) reproduces the shape at small scale: 50 parts × 400 files — whole-table list 2520 KiB vs 104 KiB max for one part at a time.Verification
GOFLAGS= make test(vet + unit)RUN_TESTS='TestProjections|TestFTP' ./test/integration/run.sh— coversskipProjectionsandcompression_format: none/tar upload paths🤖 Generated with Claude Code