github-actions[bot] commented on code in PR #65058:
URL: https://github.com/apache/doris/pull/65058#discussion_r3533868467
##########
be/src/runtime/query_context.cpp:
##########
@@ -123,6 +125,16 @@ QueryContext::QueryContext(TUniqueId query_id, ExecEnv*
exec_env,
_query_id, query_options.file_cache_query_limit_percent);
}
+ const bool initialize_remote_scan_cache_write_limiter =
Review Comment:
This creates the limiter only for normal fragment execution through
`QueryContext`, but `file_cache_query_limit_bytes` is also serialized into
short-circuit point-query requests and top-N row-id fetch still reads internal
Doris files through storage readers without a `QueryContext`. In those paths BE
only builds a lightweight `RuntimeState` (`PointQueryExecutor::Reusable::init`)
or a `PMultiGetRequestV2` carrying `file_cache_remote_only_on_miss`; the
column-store fallback `StorageReadOptions` never gets a
`RemoteScanCacheWriteLimiter`, so `file_cache_query_limit_bytes = 0` can still
write file-cache blocks there. Please create or propagate an equivalent
per-request limiter into those `StorageReadOptions`/row-store reads so the
session limit applies to all SELECT read paths.
##########
be/src/io/io_common.h:
##########
@@ -194,6 +213,7 @@ struct IOContext {
// if true, bypass peer read / peer-vs-S3 race and read directly from
remote storage
bool bypass_peer_read {false};
FileCacheMissPolicy file_cache_miss_policy =
FileCacheMissPolicy::READ_THROUGH_AND_WRITE_BACK;
+ RemoteScanCacheWriteLimiter* remote_scan_cache_write_limiter = nullptr; //
Ref
Review Comment:
This raw ref needs a lifetime rule for copied `IOContext`s.
`CachedRemoteFileReader::prefetch_range()` copies the whole `IOContext` into an
async dry-run task and clears `query_id`, `file_cache_stats`, and
`file_reader_stats`, but it will now also copy this
`remote_scan_cache_write_limiter` pointer. The limiter is owned by
`QueryContext`, so if the query finishes before the prefetch task runs, the
task can dereference a freed limiter via `use_remote_only_on_cache_miss()` /
`CacheContext`. Please either clear this pointer for async prefetch copies or
keep the limiter alive with shared ownership for the task.
--
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]
