-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[fix](io) Harden Parquet reader-local cache for File Scanner V2 #66548
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
base: branch-4.1
Are you sure you want to change the base?
Changes from all commits
614d5bc
3014486
17857e1
9d157b9
12a6cea
57c1d27
cecb633
33fcdf2
b2e5d97
721924f
dde77c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,7 @@ | |
| #include "format_v2/table_reader.h" | ||
| #include "format_v2/wal/wal_table_reader.h" | ||
| #include "io/cache/block_file_cache_profile.h" | ||
| #include "io/cache/cached_remote_file_reader.h" | ||
| #include "io/fs/file_meta_cache.h" | ||
| #include "io/io_common.h" | ||
| #include "runtime/descriptors.h" | ||
|
|
@@ -281,6 +282,14 @@ Status adapt_runtime_filter_for_table_reader(VExprSPtr* expr) { | |
|
|
||
| } // namespace | ||
|
|
||
| int64_t FileScannerV2::_cumulative_profile_delta(int64_t current, int64_t* reported) { | ||
| DORIS_CHECK(reported != nullptr); | ||
| DORIS_CHECK(current >= *reported); | ||
| const int64_t delta = current - *reported; | ||
| *reported = current; | ||
| return delta; | ||
| } | ||
|
|
||
| #ifdef BE_TEST | ||
| FileScannerV2::FileScannerV2(RuntimeState* state, RuntimeProfile* profile, | ||
| std::unique_ptr<format::TableReader> table_reader) | ||
|
|
@@ -956,6 +965,12 @@ Status FileScannerV2::_to_file_format(TFileFormatType::type format_type, | |
|
|
||
| Status FileScannerV2::_init_io_ctx() { | ||
| _io_ctx = create_file_scan_io_context(_state); | ||
| if (config::enable_file_scanner_v2_reader_local_cache) { | ||
| const size_t capacity = cast_set<size_t>( | ||
| std::max<int64_t>(0, config::file_scanner_v2_reader_local_cache_size)); | ||
| _io_ctx->reader_local_cache = std::make_shared<io::FileScannerV2ReaderLocalCache>( | ||
| capacity, _state->query_mem_tracker()); | ||
| } | ||
| return Status::OK(); | ||
| } | ||
|
|
||
|
|
@@ -1095,7 +1110,10 @@ void FileScannerV2::update_realtime_counters() { | |
| _state->get_query_ctx()->resource_ctx()->io_context()->update_scan_bytes_from_remote_storage( | ||
| deltas.scan_bytes_from_remote_storage); | ||
|
|
||
| COUNTER_SET(_file_read_bytes_counter, bytes_read); | ||
| // Scanner instances share the profile counter, so publishing an absolute value would erase | ||
| // bytes already reported by sibling scanners. | ||
| COUNTER_UPDATE(_file_read_bytes_counter, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Aggregate calls and time across sibling scanners too The sharing rule in this new comment applies to all three adjacent counters, but only bytes was converted to a per-scanner additive delta. |
||
| _cumulative_profile_delta(bytes_read, &_reported_file_read_bytes)); | ||
| COUNTER_SET(_file_read_calls_counter, cast_set<int64_t>(_file_reader_stats->read_calls)); | ||
| COUNTER_SET(_file_read_time_counter, cast_set<int64_t>(_file_reader_stats->read_time_ns)); | ||
|
|
||
|
|
@@ -1192,7 +1210,9 @@ void FileScannerV2::_collect_profile_before_close() { | |
| _reported_file_cache_statistics = *_file_cache_statistics; | ||
| } | ||
| if (_file_reader_stats != nullptr) { | ||
| COUNTER_SET(_file_read_bytes_counter, cast_set<int64_t>(_file_reader_stats->read_bytes)); | ||
| COUNTER_UPDATE(_file_read_bytes_counter, | ||
| _cumulative_profile_delta(cast_set<int64_t>(_file_reader_stats->read_bytes), | ||
| &_reported_file_read_bytes)); | ||
| COUNTER_SET(_file_read_calls_counter, cast_set<int64_t>(_file_reader_stats->read_calls)); | ||
| COUNTER_SET(_file_read_time_counter, cast_set<int64_t>(_file_reader_stats->read_time_ns)); | ||
| const auto read_time = cast_set<int64_t>(_file_reader_stats->read_time_ns); | ||
|
|
||
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.
[P2] Keep the benchmark validation guide in sync
This loop now registers 19 x 7 x 2 = 266 decoder cases, and the changed invariant test expects 266, but
be/benchmark/parquet/AGENTS.mdstill tells reviewers to expect 228 in three places and still lists only six selectivities. That guide is the mandatory validation contract for this directory, so its prescribed registration check will reject the new matrix. Please update the count and add the 5% axis there as part of this change.