github-actions[bot] commented on code in PR #65058:
URL: https://github.com/apache/doris/pull/65058#discussion_r3534526543
##########
be/src/storage/segment/variant/variant_column_reader.cpp:
##########
@@ -252,7 +252,7 @@ Status VariantColumnReader::_create_hierarchical_reader(
ColumnIteratorUPtr* reader, int32_t col_uid, PathInData path,
const SubcolumnColumnMetaInfo::Node* node, const
SubcolumnColumnMetaInfo::Node* root,
ColumnReaderCache* column_reader_cache, OlapReaderStatistics* stats,
- HierarchicalDataIterator::ReadType read_type) {
+ HierarchicalDataIterator::ReadType read_type, const io::IOContext*
io_ctx) {
// make sure external meta is loaded otherwise can't find any meta data
for extracted columns
// TODO(lhy): this will load all external meta if not loaded, and memory
will be consumed.
RETURN_IF_ERROR(load_external_meta_once());
Review Comment:
`file_cache_query_limit_segment_meta=true` still misses externalized
variant/column metadata. This method receives the query `IOContext`, but
`load_external_meta_once()` does not take it; the downstream
`VariantExternalMetaReader` calls `ExternalColMetaUtil::read_col_meta()`, which
builds a fresh `io::IOContext {.is_index_data = true}` with no
`ReaderType::READER_QUERY`, stats, or `remote_scan_cache_write_limiter`.
`ColumnMetaAccessorV3::traverse_metas()` does the same for the full external
meta region. Because `CacheContext` only consults the limiter for query
readers, a query with `file_cache_query_limit_bytes = 0` can still write these
external segment-metadata blocks into file cache. Please thread the source
query context through the external-meta readers/read helpers, using the same
derived index-metadata context as footer/column-reader metadata.
##########
be/src/service/point_query_executor.cpp:
##########
@@ -528,10 +543,14 @@ Status PointQueryExecutor::_lookup_row_data() {
// fill block by row store
if (_reusable->rs_column_uid() != -1) {
bool use_row_cache = !config::disable_storage_row_cache;
+ io::IOContext io_ctx;
Review Comment:
`file_cache_query_limit_bytes` is still not applied to the point-query
row-key lookup. This new limiter is attached only inside `_lookup_row_data()`,
but `_lookup_row_key()` runs earlier and calls `_tablet->lookup_row_key(...)`
without an `IOContext`. That path loads segments with
`need_load_pk_index_and_bf=true`, and `Segment::lookup_row_key()` also calls
`load_pk_index_and_bf(stats)` without the source context, so primary-key
index/Bloom-filter metadata can still be written to file cache before the
point-query limiter is ever consulted. Please propagate the point-query
`IOContext` through `BaseTablet::lookup_row_key()` /
`SegmentLoader::load_segments()` / `Segment::lookup_row_key()` as well, so
`file_cache_query_limit_bytes = 0` really suppresses all point-query remote
cache writes.
##########
be/src/io/cache/file_cache_common.h:
##########
@@ -163,6 +164,13 @@ struct CacheContext {
}
query_id = io_context->query_id ? *io_context->query_id : TUniqueId();
is_warmup = io_context->is_warmup;
+ remote_scan_cache_write_limiter =
io_context->remote_scan_cache_write_limiter;
Review Comment:
This gate will not catch external file scans because their `IOContext` never
carries the two fields it now requires. `FileScannerV2::_init_io_ctx()` only
sets `query_id`, prepare-time setup only adds stats/disposable state, and that
same context is passed through `TableReader` into format readers where
`FileFactory::get_reader_options()` selects `FILE_BLOCK_CACHE` for cached
remote files. With no copied `remote_scan_cache_write_limiter` and no
`ReaderType::READER_QUERY`, a SELECT with `file_cache_query_limit_bytes = 0`
can still populate file cache through external scans. Please attach the query
limiter from `RuntimeState::get_query_ctx()` and mark these query scan contexts
as `READER_QUERY` where appropriate, preserving existing non-query/load
semantics.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
