Skip to content

fix: snapshot cli: reduce memory footprint#179

Merged
Christos Vontas (cv711) merged 15 commits intomainfrom
fix/memory-footprint
Mar 5, 2026
Merged

fix: snapshot cli: reduce memory footprint#179
Christos Vontas (cv711) merged 15 commits intomainfrom
fix/memory-footprint

Conversation

@cv711
Copy link
Copy Markdown
Contributor

@cv711 Christos Vontas (cv711) commented Mar 3, 2026

This pull request introduces several improvements and refactorings to the S3 backup and restore logic, focusing on concurrency handling, worker configuration, and error management. The main changes include refactoring the upload and download worker pools for S3 operations, improving error handling to avoid deadlocks, and adding comprehensive tests for the new download worker logic. Additionally, there are some minor fixes and improvements to command-line naming and logging.

S3 Worker Pool Refactoring and Concurrency Improvements:

  • Refactored the S3 upload (uploadBucket) and download (DownloadBucket) logic to use configurable worker pools, improving performance and reliability for large-scale operations. This includes introducing totalUploadWorkers and totalDownloadWorkers constants, and splitting worker allocation between public and private buckets. The new implementation uses generic worker pool functions (runWorkers, runUploadWorkers, runDownloadWorkers) to process files and objects concurrently, with improved cancellation and error propagation. [1] [2] [3] [4] [5] [6] [7] [8]
  • Added a new test suite (internal/util/s3_downloader_test.go) covering the download worker pool implementation. Tests verify correct concurrency, error handling, and ensure no deadlocks occur if workers or producers fail. Benchmarks were also added to measure memory usage and performance for different worker counts.

Error Handling and Logging Enhancements:

  • Improved error reporting and cancellation in worker pools, ensuring that errors from workers or producers are propagated correctly and that all goroutines exit cleanly, preventing resource leaks or deadlocks. [1] [2]
  • Enhanced logging in the MySQL shell dump logic to only log the tail of large outputs, reducing log noise and improving debuggability. [1] [2]

Miscellaneous Improvements and Fixes:

  • Fixed a typo in the CLI command name (snpashotsnapshot) in cmd/snapshot/snapshot.go.
  • Improved progress logging and statistics for S3 uploads, now reporting both scanned and uploaded file counts.

These changes collectively make the S3 backup and restore process more robust, scalable, and easier to maintain.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors S3 backup/restore concurrency to reduce memory usage by introducing bounded worker pools for uploads/downloads, making worker counts explicit, and adding tests/benchmarks for the new helpers.

Changes:

  • Introduce explicit worker-count constants and split upload/download concurrency across public/private buckets.
  • Replace ad-hoc goroutine/semaphore logic with generic worker helpers (runWorkers, runDownloadWorkers, runUploadWorkers) and improved progress reporting.
  • Add unit tests and benchmarks for the new upload/download worker helpers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
internal/util/s3.go Adds explicit upload worker constant, refactors bucket upload and downloader to use new generic worker helpers.
internal/snapshot/s3.go Adds explicit download worker constant and uses it (split across buckets) during snapshot asset backup.
internal/util/s3_downloader_test.go New tests/benchmarks for bounded download worker behavior and error handling.
internal/util/s3_uploader_test.go New tests/benchmarks for bounded upload worker behavior and error handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/util/s3.go
Comment thread internal/util/s3.go Outdated
Comment thread internal/util/s3.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/util/s3.go Outdated
Comment thread internal/util/s3.go
Comment thread internal/util/s3.go
Comment thread internal/util/s3.go Outdated
Christos Vontas (cv711) and others added 2 commits March 4, 2026 14:32
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread internal/util/s3.go Outdated
Comment thread internal/snapshot/s3.go
@cv711 Christos Vontas (cv711) merged commit a513ca4 into main Mar 5, 2026
4 checks passed
@cv711 Christos Vontas (cv711) deleted the fix/memory-footprint branch March 5, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants