feat(csharp/src/Drivers/Databricks): Clarify CloudFetch memory manager behavior and set appropriate limit #3656
Conversation
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
…on of cloud downloads (apache#3652)" This reverts commit eae39dd.
…nd reduce default limit The CloudFetchMemoryBufferManager tracks in-flight download memory based on compressed file sizes, not decompressed sizes. This is intentional, as: 1. The memory manager limits concurrent downloads to prevent unbounded resource usage 2. Decompressed data memory is naturally bounded by the result queue capacity and batch processing flow 3. Tracking compressed sizes provides a lightweight mechanism to control download concurrency without the overhead of tracking decompressed memory The default memory limit has been reduced from 200MB to 100MB to provide better control over concurrent download behavior. With typical file sizes of ~1MB compressed, this allows for approximately 100 concurrent in-flight downloads. Changes: - Added detailed documentation to CloudFetchMemoryBufferManager explaining it tracks in-flight compressed data sizes - Reduced DefaultMemoryBufferSizeMB from 200 to 100 in CloudFetchDownloadManager - Added inline comments clarifying that size parameters represent compressed file sizes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ion of cloud downloads (apache#3652)" This reverts commit dae9c27.
eric-wang-1990
changed the title
CurtHagenlocher
changed the title
CurtHagenlocher
changed the title
Contributor
|
Thanks, @eric-wang-1990! The comments say "Verify the original regression ... was resolved" but the code doesn't reflect either that the previous change was reapplied or that it was tested. Assuming the code reflects what you actually want to check in, could you edit the comment to match it please? |
Contributor
Author
Done |
CurtHagenlocher
approved these changes
Oct 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Clarifies memory manager behavior - Documents that
CloudFetchMemoryBufferManagertracks in-flight compressed download sizes and reduces the default from 200MB to 100MBMemory Manager Clarification
The
CloudFetchMemoryBufferManagertracks in-flight download memory based on compressed file sizes, not decompressed sizes. This design is intentional:Changes
CloudFetchMemoryBufferManagerexplaining it tracks in-flight compressed data sizesDefaultMemoryBufferSizeMBfrom 200 to 100 inCloudFetchDownloadManagerTest plan
🤖 Generated with Claude Code