github-actions[bot] commented on code in PR #65058:
URL: https://github.com/apache/doris/pull/65058#discussion_r3535874444
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -5784,7 +5795,7 @@ public TQueryOptions toThrift() {
tResult.setIcebergWriteTargetFileSizeBytes(icebergWriteTargetFileSizeBytes);
tResult.setEnableLocalShufflePlanner(enableLocalShufflePlanner);
-
+ tResult.setFileCacheQueryLimitBytes(fileCacheQueryLimitBytes);
Review Comment:
`file_cache_query_limit_bytes` also needs to be refreshed for reusable
prepared short-circuit queries. The first short-circuit execute serializes
`planner.getQueryOptions()` into
`ShortCircuitQueryContext.serializedQueryOptions`; later reusable executes
return through `directExecuteShortCircuitQuery()` and either resend those
cached options or just the cached UUID, while BE creates the limiter from the
cached `_reusable->runtime_state()->query_options()`. If a session
prepares/executes a point query with the default `-1`, then runs `SET
file_cache_query_limit_bytes=0` and executes the same prepared statement again,
the reused BE context still sees `-1` and `_remote_scan_cache_write_limiter`
stays null, so remote point-query reads can still populate file cache. Please
refresh the query options for reusable short-circuit executes, include the
current byte limit in the lightweight request, or invalidate the cached
short-circuit context when this session variable changes.
##########
be/src/storage/segment/variant/variant_external_meta_reader.cpp:
##########
@@ -90,7 +90,9 @@ Status
VariantExternalMetaReader::init_from_footer(std::shared_ptr<const Segment
}
Status VariantExternalMetaReader::lookup_meta_by_path(const std::string&
rel_path,
- ColumnMetaPB* out_meta)
const {
+ ColumnMetaPB* out_meta,
+ OlapReaderStatistics*
stats,
+ const io::IOContext*
source_io_ctx) const {
Review Comment:
This still only applies the source context to the value-meta reads. The key
side of the external variant metadata remains outside the limiter:
`init_from_footer()` loads `_key_reader` with `_key_reader->load(true, false)`,
and `lookup_meta_by_path()`, `load_all()`, and `has_prefix()` all build
`IndexedColumnIterator(_key_reader.get())` without `stats` or `source_io_ctx`.
Those calls reach `IndexedColumnReader::read_page()` with a default
`IOContext`, so the `variant_meta_keys` index/data pages are `is_index_data`
but not `READER_QUERY` and have no `remote_scan_cache_write_limiter`. With
`file_cache_query_limit_segment_meta=true` and
`file_cache_query_limit_bytes=0`, reading or scanning external variant keys can
still populate file cache before the value-meta reads are limited. Please
thread the source context/stats into the key reader load and key iterators too.
##########
be/src/storage/index/inverted/inverted_index_fs_directory.cpp:
##########
@@ -184,10 +184,14 @@ void DorisFSDirectory::FSIndexInput::setIoContext(const
void* io_ctx) {
_io_ctx.reader_type = ctx->reader_type;
_io_ctx.query_id = ctx->query_id;
_io_ctx.file_cache_stats = ctx->file_cache_stats;
+ _io_ctx.file_cache_miss_policy = ctx->file_cache_miss_policy;
+ _io_ctx.remote_scan_cache_write_limiter =
ctx->remote_scan_cache_write_limiter;
Review Comment:
Copying the limiter here only helps callers that put it into the
`IOContext`, but the score-runtime statistics path still builds one without it.
In `OlapScanner`, the `CollectionStatistics::collect()` context sets
`reader_type`, `query_id`, stats, and `is_inverted_index=true`, but never
copies `state->get_query_ctx()->remote_scan_cache_write_limiter()`.
`CollectionStatistics::process_segment()` then opens and reads the
inverted-index files through this context, so `FSIndexInput::setIoContext()`
copies a null limiter and `CacheContext` will keep admitting cache writes even
when `file_cache_query_limit_bytes=0`. Please attach the query limiter to that
score-runtime collection context as well.
--
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]
