github-actions[bot] commented on code in PR #69768:
URL: https://github.com/apache/doris/pull/69768#discussion_r4476737337
##########
gensrc/thrift/PaloInternalService.thrift:
##########
@@ -544,6 +544,9 @@ struct TQueryOptions {
// reads the data instead.
// Defaults to false because an old FE never sends this field, and BE
checked both cases before.
1006: optional bool force_pushdown_zonemap_minmax = false
+ // Use the cache option range to avoid unrelated query-option IDs allocated
on other branches.
+ // Explicitly sent by new FEs; an omitted field keeps older-FE queries on
the local policy.
+ 1007: optional bool enable_file_cache_io_adaptor = false;
Review Comment:
[P1] Allocate field 1007 on master before this release pick
`TQueryOptions` adds field 1007 here, but the same field is absent from
current master and every inspected release branch. The gensrc compatibility
guide requires new IDs to land on master first and then be picked with the same
name, type, and meaning; a release-only allocation can collide with a future
master field or pick. Add/reserve 1007 on master first, then carry that exact
definition into this branch.
##########
be/src/io/cache/cached_remote_file_reader.cpp:
##########
@@ -1060,15 +1053,11 @@ Status
CachedRemoteFileReader::_read_remaining_blocks_from_cache(
size_t remote_bytes_read {0};
stats.hit_cache = false;
stats.from_peer_cache = false;
- s3_read_counter << 1;
- SCOPED_RAW_TIMER(&stats.remote_read_timer);
- RETURN_IF_ERROR(_remote_file_reader->read_at(
+ RETURN_IF_ERROR(_read_remote_at(
current_offset, Slice(result.data + (current_offset -
offset), read_size),
- &remote_bytes_read, io_ctx));
+ &remote_bytes_read, io_ctx, stats));
Review Comment:
[P2] Train the remote history for the same size used by full-hit routing
A normal 64 KiB external read with 1 MiB cache blocks is aligned by
`s_align_size()` and fetched remotely as a 1 MiB span, so `_read_remote_at()`
records that sample in the 1 MiB bucket here. Once cached, the same 64 KiB
request is routed through `_try_read_with_io_adaptor()`, which selects the 64
KiB bucket. That bucket has local samples but no remote samples, so the
controller never reaches its 50/50-window bypass decision and the new adaptor
remains inactive for partial-block reads. Record or normalize miss latency
using the user-range bucket, or select using the physical block span
consistently, so a cold-to-hot sequence can compare like-for-like service times.
##########
regression-test/suites/query_p0/virtual_slot_ref/adjust_virtual_slot_nullable.groovy:
##########
@@ -20,9 +20,11 @@ suite("adjust_virtual_slot_nullable") {
def tbl1 = "tbl_adjust_virtual_slot_nullable_1"
def tbl2 = "tbl_adjust_virtual_slot_nullable_2"
+ // Cost-based join commutation changes the recorded shape when cloud
statistics arrive.
+ // Keep the nullable-slot assertion independent of statistics collection
timing.
multi_sql """
set runtime_filter_mode='OFF';
- set disable_join_reorder=false;
+ set disable_join_reorder=true;
Review Comment:
[P2] Keep a reordered plan in the nullable-slot regression
This test originally set `disable_join_reorder=false`, so it exercised the
optimizer's join-commutation path while checking
`adjust_virtual_slot_nullable`. Changing it to `true` removes that path
entirely; a nullable virtual-slot bug that only appears after commutation can
now regress without failing this test. Keep a deterministic reordered case, or
add a second case with shape normalization, while stabilizing the
statistics-sensitive output.
--
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]
