-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](cloud) Fix warm up cache not supporting packed file #60375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix](cloud) Fix warm up cache not supporting packed file #60375
Conversation
|
run buildall |
There was a problem hiding this 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 fixes a bug where warm-up cache functionality fails when enable_packed_file is enabled. The issue occurs because the code was using the raw FileSystem from storage_resource->fs instead of the wrapped FileSystem from RowsetMeta::fs(), which properly handles packed files by wrapping the FileSystem with PackedFileSystem.
Changes:
- Updated three warm-up download functions to use
RowsetMeta::fs()instead ofstorage_resource->fsfor proper packed file support - Modified 17 regression tests to randomly enable/disable
enable_packed_fileconfiguration to ensure both scenarios are tested
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| be/src/io/cache/block_file_cache_downloader.cpp | Updated download_file_cache_block to use RowsetMeta::fs() with null check for packed file support |
| be/src/cloud/cloud_tablet.cpp | Updated _submit_segment_download_task and _submit_inverted_index_download_task to use rowset_meta->fs() with null checks |
| regression-test/suites/cloud_p0/tablets/test_clean_tablet_when_rebalance.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/tablets/test_clean_tablet_when_drop_force_table.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/tablets/test_clean_stale_rs_index_file_cache.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/tablets/test_clean_stale_rs_file_cache.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/cache/test_topn_broadcast.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/cache/test_load_cache.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_warmup_rebalance.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_peer_read_async_warmup.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_expanding_node_balance.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_warm_up_with_compaction_use_peer_cache.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_warm_up_use_peer_cache.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_warm_up_task_abnormal.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_warm_up_sync_global_config.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_warm_up.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_use_compute_group_properties.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_balance_metrics.groovy | Added random enable_packed_file configuration to test both scenarios |
| regression-test/suites/cloud_p0/balance/test_alter_compute_group_properties.groovy | Added random enable_packed_file configuration to test both scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 31842 ms |
ClickBench: Total hot run time: 28.28 s |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
8fc190e to
78aa79c
Compare
|
run buildall |
78aa79c to
467d9ac
Compare
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
TPC-H: Total hot run time: 31738 ms |
ClickBench: Total hot run time: 28.75 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
When enable_packed_file is enabled, the warm up functions fail to download cache data because they use storage_resource->fs (the raw FileSystem) instead of RowsetMeta::fs() which wraps the FileSystem with PackedFileSystem. The PackedFileSystem correctly maps segment file paths to their actual locations within packed files. Without this mapping, the download fails because the segment files don't exist on S3 - they are packed into larger packed files. This fix changes the file_system used in: 1. download_file_cache_block (block_file_cache_downloader.cpp) 2. _submit_segment_download_task (cloud_tablet.cpp) 3. _submit_inverted_index_download_task (cloud_tablet.cpp) All now use RowsetMeta::fs() instead of storage_resource->fs to properly handle packed files.
467d9ac to
703d692
Compare
|
run buildall |
TPC-H: Total hot run time: 32221 ms |
ClickBench: Total hot run time: 28.35 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
PR approved by at least one committer and no changes requested. |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
When enable_packed_file is enabled, the warm up functions fail to download cache data because they use storage_resource->fs (the raw FileSystem) instead of RowsetMeta::fs() which wraps the FileSystem with PackedFileSystem.
The PackedFileSystem correctly maps segment file paths to their actual locations within packed files. Without this mapping, the download fails because the segment files don't exist on S3 - they are packed into larger packed files.
This fix changes the file_system used in:
All now use RowsetMeta::fs() instead of storage_resource->fs to properly handle packed files.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)