8000 Keep the link busy until the end of a transfer: shared download budget, file-level streams, GCS ranged reads by Slach · Pull Request #1601 · Altinity/clickhouse-backup · GitHub
[go: up one dir, main page]
Skip to content

Keep the link busy until the end of a transfer: shared download budget, file-level streams, GCS ranged reads - #1601

Merged
Slach merged 5 commits into
masterfrom
transfer_throughput_1454_1591_1592
Oct 8, 2026
Merged

Keep the link busy until the end of a transfer: shared download budget, file-level streams, GCS ranged reads#1601
Slach merged 5 commits into
masterfrom
transfer_throughput_1454_1591_1592

Conversation

@Slach
@Slach Slach commented Oct 7, 2026
Copy link
Copy Markdown
Collaborator

Fixes #1591, fixes #1454, fixes #1592

All three issues are about the same thing: keeping the link busy until the end of a transfer. #1454 and #1591 share one mechanism: the global transfer budget from #1591 is the "product bounded by a global semaphore" that the #1454 review asked for. #1592 is a separate GCS storage-layer change. It is opt-in, kept as the author's own commit, and can be dropped from this PR on its own.

Commits

  1. feat(gcs): stream large archives as concurrent byte ranges (GCS download: one large archive is always a single stream at the tail; allow_multipart_download needs a temp file and is bypassed by download_max_bytes_per_second - ranged streaming reader, benchmark and branch #1592, @minguyen9988, cherry-picked unchanged from minguyen9988:fix-gcs-ranged-streaming-download)
    • Archives of at least gcs.ranged_download_min_size are read as gcs.ranged_download_concurrency concurrent NewRangeReader requests and fed to decompression in order.
    • No temporary file, and it works with download_max_bytes_per_second.
    • Memory is capped process-wide by ranged_download_max_buffers * ranged_download_chunk_size.
    • Disabled by default.
  2. Share one download transfer budget across tables, largest first (download: the last tables drain with only download_concurrency streams (long tail) - shared transfer budget + largest-first dispatch, benchmark and patch #1591, ported from the patch in the issue)
    • general.download_transfer_concurrency: one semaphore.Weighted per Download(), shared by every part, archive and required-part transfer of all tables.
    • The per-table part errgroup limit is raised to the budget, so the semaphore alone bounds the total.
    • 0 (default) = download_concurrency^2, the same connection maximum as today. Negative = legacy per-table limit.
    • general.download_table_order: largest_first (default) or metadata. Only the dispatch order changes; restore order is not affected.
  3. Transfer files of one part by several streams within the budget (upload/download transfers files within a part sequentially — add file-level parallelism (file_transfer_concurrency) #1454, the shape proposed in the review)
    • general.file_transfer_concurrency (default 1 = current sequential behavior) is a worker count inside the existing UploadPath / DownloadPath / DownloadPathWithManifest. No new *Parallel copies.
    • The caller goroutine is the first stream and already holds its slot.
    • Extra streams run only when TryAcquire on the same budget succeeds, so they never block and never raise the total above the budget. They only use slots left idle by other parts, mostly at the tail.
    • Budget: download_transfer_concurrency for download, upload_concurrency^2 for upload. When the option is enabled, upload parts take a slot too.
    • The first error is recorded before the other streams are canceled, so context canceled cannot hide it (a unit test caught this).
    • Only compression_format: none is affected.
  4. Docs: ReadMe entries for the gcs.ranged_download_* settings and the testflows default-config snapshot.
  5. Cap the FTP download budget by its connection pool
    • A recursive FTP Walk holds one pooled connection while its callbacks borrow another.
    • With the shared budget, a single table can now reach download_concurrency^2 transfers and exhaust the pool of 4*ftp.concurrency connections.
    • The budget is now capped at 2*ftp.concurrency.

The storage functions in commit 3 are re-indented into closures, so review with git diff -w.

Tests

  • New unit tests:
    • Budget resolution, including the FTP cap.
    • The budget bounds total transfers, and one table can reach the whole budget.
    • Cancellation, and dispatch order (stable, with the TotalBytes fallback).
    • UploadPath / DownloadPath / DownloadPathWithManifest with file workers: peak streams equal min(workers, budget), content and byte counts are exact, slots are released, and the first error is not masked. All pass under -race.
    • The author's ranged reader tests.
  • make test passes.
  • The integration SFTP config (compression_format: none) now sets file_transfer_concurrency: 4.
  • Integration tests passed locally: TestS3, TestFTP, TestSFTPAuthPassword, TestBwLimitSFTP, TestRebaseSFTP. largest_first and the auto budget are on by default, so every download test also exercises them.

Open points for review

🤖 Generated with Claude Code

minguyen9988 and others added 5 commits October 7, 2026 08:05
@minguyen9988 @Slach
feat(gcs): stream large archives as concurrent byte ranges (ranged_do…
d682475 
…wnload_*)

One GCS read stream has a fixed ceiling, so when a download is down to its
last large part the link is mostly idle. allow_multipart_download covers this
only partly: it buffers the whole object into a temporary file before
extraction starts (extra disk equal to the object size per file in flight),
and it is bypassed whenever download_max_bytes_per_second is set (#1377).

Add an opt-in ranged streaming reader:

- objects >= gcs.ranged_download_min_size are read as
  gcs.ranged_download_concurrency concurrent NewRangeReader requests of
  gcs.ranged_download_chunk_size bytes and handed to the existing
  decompress/untar pipeline strictly in order; no temporary file
- memory is bounded per reader (a window of `concurrency` chunks) and
  process-wide (gcs.ranged_download_max_buffers chunk buffers shared by all
  readers), so at the tail the last large file gets the whole budget
- chunk indexes are claimed in order by goroutines already holding both
  slots, so readers sharing the budget cannot deadlock
- each range borrows its own pooled client for the duration of the request,
  so ranges are not multiplexed onto one HTTP/2 connection
- compatible with download_max_bytes_per_second: the limiter paces the
  consumer and the prefetch stays bounded
- disabled by default (ranged_download_concurrency: 0)
@Slach @claude
Share one download transfer budget across tables, largest first
63ec7ab 
During download the tables are dispatched in metadata order, at most
download_concurrency at a time, and each table runs its parts with its
own download_concurrency limit. When only one table is left it can use
only its own slots, so the tail of a download drains with
download_concurrency streams while the slots of finished tables stay idle.

- general.download_transfer_concurrency: one semaphore per Download()
  shared by every part/archive/required-part transfer of all tables; the
  per-table part limit is raised to the budget, so the semaphore alone
  bounds the total. 0 (default) = download_concurrency^2, the current
  worst case, so no extra connections; negative = legacy per-table limit
- general.download_table_order: largest_first (default) dispatches big
  tables first, so they are not left to drain alone at the end; metadata
  keeps the old order. Only the dispatch order changes. The cost is the
  sum of per-part sizes (2.8.0+), TotalBytes for older backups

Ported from the patch in #1591 by @minguyen9988.

Fixes #1591

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Slach @claude
Transfer files of one part by several streams within the budget
1b0feee 
With compression_format: none UploadPath, DownloadPath and
DownloadPathWithManifest move the files of one part one by one, so on
high-latency remote storage per-file round trips dominate a wide part.

general.file_transfer_concurrency (default 1 = sequential) lets one path
transfer use up to N streams. The caller goroutine is the first stream
and holds its own slot of the transfer budget; every extra stream runs
only while it can take a free slot of the same budget without blocking
(TryAcquire), so the total number of streams never exceeds the budget:
download_transfer_concurrency for download, upload_concurrency^2 for
upload (upload parts now take a slot too when the option is enabled).
Extra streams just use the slots left idle by other parts, typically at
the tail of a transfer, and the product parts x files is bounded as
requested in the #1454 review. The first error is recorded before the
other streams are canceled, so it is not hidden by `context canceled`.

The integration SFTP config enables it with 4 streams.

Fixes #1454

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Slach @claude
Document gcs ranged_download_* settings, update default config snapshot
5d5b1b0 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Slach @claude
Cap the FTP download budget by its connection pool
d1296e6 
A recursive FTP Walk keeps one pooled connection while its callbacks
borrow another one. With a shared budget a single table can now run
download_concurrency^2 part transfers, which can exhaust the pool of
4*ftp.concurrency connections and hang; limit the budget to
2*ftp.concurrency.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coveralls
coveralls commented Oct 7, 2026
Copy link
Copy Markdown

Coverage Report for CI Build 37576468474

Coverage increased (+0.1%) to 69.593%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: 121 uncovered changes across 7 files (341 of 462 lines covered, 73.81%).
  • 26 coverage regressions across 9 files.

Uncovered Changes

File Changed Covered %
pkg/storage/gcs.go 65 9 13.85%
pkg/storage/general.go 85 62 72.94%
pkg/storage/manifest.go 49 30 61.22%
pkg/storage/ranged_reader.go 133 122 91.73%
pkg/backup/download.go 29 23 79.31%
pkg/backup/upload.go 19 15 78.95%
pkg/config/config.go 10 8 80.0%
Total (9 files) 462 341 73.81%

Coverage Regressions

26 previously-covered lines in 9 files lost coverage.

File Lines Losing Coverage Coverage
p 8000 kg/storage/general.go 5 70.26%
pkg/backup/backuper.go 4 77.54%
pkg/backup/create.go 4 75.2%
pkg/clickhouse/clickhouse.go 4 80.69%
pkg/backup/create_remote.go 2 66.39%
pkg/backup/download.go 2 78.09%
pkg/storage/azblob.go 2 63.61%
pkg/storage/object_disk/object_disk.go 2 70.14%
pkg/storage/manifest.go 1 58.72%

Coverage Stats

Coverage Status
Relevant Lines: 28253
Covered Lines: 19662
Line Coverage: 69.59%
Coverage Strength: 35145.41 hits per line
💛 - Coveralls

@Slach
Slach merged commit bd55fa2 into master Oct 8, 2026
30 checks passed
@Slach Slach mentioned this pull request Oct 8, 2026
Open
@minguyen9988 minguyen9988 mentioned this pull request Oct 10, 2026
Closed
@guybrush guybrush mentioned this pull request Oct 22, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
0