From b4294bb5dba96c044c13e542a65bd4e39722204e Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Sat, 11 Jul 2026 20:14:21 +0800 Subject: [PATCH 1/9] [feature](query-cache) Support incremental merge for stale query cache entries Hourly batch loads keep bumping the version of the hot partition, so its query cache entries never hit again and every "last N days" aggregation recomputes the whole partition each hour. This change lets BE reuse a stale entry instead of discarding it: scan only the delta rowsets in (cached_version, current_version], emit their partial aggregation state side by side with the cached partial blocks (the upstream merge aggregation combines both), and write the merged entry back under the new version. Correctness rests on two properties: append-only snapshots decompose as S(v2) = S(v1) + delta, and partial aggregation states merge homomorphically. Every precondition guards one of them, and any violation falls back to a full recompute silently: - FE only sets TQueryCacheParam.allow_incremental (new optional field 8) when the experimental session switch enable_query_cache_incremental is on, the cache point is a non-finalize aggregation directly above the olap scan node, and the selected index is append-only: DUP_KEYS, or merge-on-write UNIQUE_KEYS. - BE re-validates per tablet at decision time: local storage mode, version order, the compaction threshold (query_cache_max_incremental_merge_count, BE config, default 8), keys type, delta capturability, no delete predicates in the delta, and for merge-on-write tables a delete-bitmap window check that rejects any load that rewrote pre-existing keys, so an occasional backfill costs exactly one full recompute and re-bases the entry. A new fragment-level QueryCacheRuntime makes one idempotent decision (HIT / INCREMENTAL / MISS) per instance, pins the entry and pre-captures the delta read source. Centralizing the decision also fixes three pre-existing defects: the scan/cache-source double-lookup race that could write back an empty poisoned entry, row-binlog scans polluting the cache, and build_cache_key failures aborting the whole query instead of degrading to uncached execution. Observability: profile fields HitCacheStale, IncrementalDeltaVersions and IncrementalFallbackReason, plus three BE metrics (query_cache_stale_hit_total, query_cache_incremental_fallback_total, query_cache_write_back_total). Benchmark (80M-row duplicate-key table, 200k-row hourly appends, group by a non-distribution column): a stale query drops from ~307ms (full recompute) to ~19ms with incremental merge, on par with an exact hit, and the cost no longer grows with the base data size. --- be/src/common/config.cpp | 6 + be/src/common/config.h | 3 + be/src/common/metrics/doris_metrics.cpp | 11 + be/src/common/metrics/doris_metrics.h | 9 + .../exec/operator/cache_source_operator.cpp | 164 +++-- be/src/exec/operator/cache_source_operator.h | 31 +- be/src/exec/operator/olap_scan_operator.cpp | 82 ++- be/src/exec/operator/olap_scan_operator.h | 15 +- .../pipeline/pipeline_fragment_context.cpp | 24 +- .../exec/pipeline/pipeline_fragment_context.h | 7 + be/src/exec/scan/olap_scanner.cpp | 2 +- be/src/exec/scan/olap_scanner.h | 4 + be/src/runtime/query_cache/query_cache.cpp | 296 +++++++++- be/src/runtime/query_cache/query_cache.h | 192 +++++- .../operator/query_cache_operator_test.cpp | 434 +++++++++++++- be/test/exec/pipeline/query_cache_test.cpp | 558 ++++++++++++++++++ .../apache/doris/planner/AggregationNode.java | 4 + .../normalize/QueryCacheNormalizer.java | 70 +++ .../org/apache/doris/qe/SessionVariable.java | 39 ++ .../planner/QueryCacheNormalizerTest.java | 90 ++- gensrc/thrift/QueryCache.thrift | 29 +- .../cache/query_cache_incremental.groovy | 183 ++++++ 22 files changed, 2161 insertions(+), 92 deletions(-) create mode 100644 regression-test/suites/query_p0/cache/query_cache_incremental.groovy diff --git a/be/src/common/config.cpp b/be/src/common/config.cpp index 9ec587bd100ec5..be67881eab92b5 100644 --- a/be/src/common/config.cpp +++ b/be/src/common/config.cpp @@ -1690,6 +1690,12 @@ DEFINE_mBool(enable_pipeline_task_leakage_detect, "false"); DEFINE_mInt32(check_score_rounds_num, "1000"); DEFINE_Int32(query_cache_size, "512"); +// Max number of incremental merges accumulated on one query cache entry before +// a full recompute is forced. Each incremental merge appends the delta partial +// blocks to the entry, so the entry gets more fragmented (and the upstream merge +// aggregation does more work) as deltas accumulate; a periodic full recompute +// compacts the entry back to a minimal set of blocks. +DEFINE_mInt32(query_cache_max_incremental_merge_count, "8"); // Enable validation to check the correctness of table size. DEFINE_Bool(enable_table_size_correctness_check, "false"); diff --git a/be/src/common/config.h b/be/src/common/config.h index 012f09a735c0cd..49a5307a205c94 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -1750,6 +1750,9 @@ DECLARE_mInt32(check_score_rounds_num); // MB DECLARE_Int32(query_cache_size); +// Max incremental merges on one query cache entry before forcing a full +// recompute to compact the entry (see query_cache.h QueryCacheRuntime). +DECLARE_mInt32(query_cache_max_incremental_merge_count); DECLARE_Bool(force_regenerate_rowsetid_on_start_error); // Enable validation to check the correctness of table size. diff --git a/be/src/common/metrics/doris_metrics.cpp b/be/src/common/metrics/doris_metrics.cpp index 85317133e12382..64d542970098eb 100644 --- a/be/src/common/metrics/doris_metrics.cpp +++ b/be/src/common/metrics/doris_metrics.cpp @@ -48,6 +48,14 @@ DEFINE_COUNTER_METRIC_PROTOTYPE_2ARG(query_scan_bytes_from_local, MetricUnit::BY DEFINE_COUNTER_METRIC_PROTOTYPE_2ARG(query_scan_bytes_from_remote, MetricUnit::BYTES); DEFINE_COUNTER_METRIC_PROTOTYPE_2ARG(query_scan_rows, MetricUnit::ROWS); DEFINE_COUNTER_METRIC_PROTOTYPE_2ARG(query_scan_count, MetricUnit::NOUNIT); +DEFINE_COUNTER_METRIC_PROTOTYPE_3ARG(query_cache_stale_hit_total, MetricUnit::REQUESTS, + "Query cache decisions that reused a stale entry by " + "incremental merge."); +DEFINE_COUNTER_METRIC_PROTOTYPE_3ARG(query_cache_incremental_fallback_total, MetricUnit::REQUESTS, + "Query cache decisions that could have merged incrementally " + "but fell back to a full recompute."); +DEFINE_COUNTER_METRIC_PROTOTYPE_3ARG(query_cache_write_back_total, MetricUnit::REQUESTS, + "Query cache entries written back (full or merged)."); DEFINE_COUNTER_METRIC_PROTOTYPE_5ARG(push_requests_success_total, MetricUnit::REQUESTS, "", push_requests_total, Labels({{"status", "SUCCESS"}})); DEFINE_COUNTER_METRIC_PROTOTYPE_5ARG(push_requests_fail_total, MetricUnit::REQUESTS, "", @@ -291,6 +299,9 @@ DorisMetrics::DorisMetrics() : _metric_registry(_s_registry_name) { INT_COUNTER_METRIC_REGISTER(_server_metric_entity, query_scan_bytes_from_local); INT_COUNTER_METRIC_REGISTER(_server_metric_entity, query_scan_bytes_from_remote); INT_COUNTER_METRIC_REGISTER(_server_metric_entity, query_scan_rows); + INT_COUNTER_METRIC_REGISTER(_server_metric_entity, query_cache_stale_hit_total); + INT_COUNTER_METRIC_REGISTER(_server_metric_entity, query_cache_incremental_fallback_total); + INT_COUNTER_METRIC_REGISTER(_server_metric_entity, query_cache_write_back_total); INT_COUNTER_METRIC_REGISTER(_server_metric_entity, push_requests_success_total); INT_COUNTER_METRIC_REGISTER(_server_metric_entity, push_requests_fail_total); diff --git a/be/src/common/metrics/doris_metrics.h b/be/src/common/metrics/doris_metrics.h index 764d5dd956a490..ac6e993698d6ad 100644 --- a/be/src/common/metrics/doris_metrics.h +++ b/be/src/common/metrics/doris_metrics.h @@ -54,6 +54,15 @@ class DorisMetrics { IntCounter* query_scan_bytes_from_remote = nullptr; IntCounter* query_scan_rows = nullptr; + // Query cache incremental merge (see runtime/query_cache/query_cache.h): + // how many instance decisions reused a stale entry incrementally, how many + // could have but fell back to a full recompute, and how many entries were + // written back. Per-query breakdown lives in the profile + // (HitCacheStale / IncrementalFallbackReason / InsertCache). + IntCounter* query_cache_stale_hit_total = nullptr; + IntCounter* query_cache_incremental_fallback_total = nullptr; + IntCounter* query_cache_write_back_total = nullptr; + IntCounter* push_requests_success_total = nullptr; IntCounter* push_requests_fail_total = nullptr; IntCounter* push_request_duration_us = nullptr; diff --git a/be/src/exec/operator/cache_source_operator.cpp b/be/src/exec/operator/cache_source_operator.cpp index ec7d947680a6f8..7182868d950a0e 100644 --- a/be/src/exec/operator/cache_source_operator.cpp +++ b/be/src/exec/operator/cache_source_operator.cpp @@ -35,11 +35,11 @@ Status CacheSourceLocalState::init(RuntimeState* state, LocalStateInfo& info) { ((DataQueueSharedState*)_dependency->shared_state()) ->data_queue.set_source_dependency(_shared_state->source_deps.front()); const auto& scan_ranges = info.scan_ranges; - bool hit_cache = false; - const auto& cache_param = _parent->cast()._cache_param; + auto& parent = _parent->cast(); + const auto& cache_param = parent._cache_param; // 1. init the slot orders - const auto& tuple_descs = _parent->cast().row_desc().tuple_descriptors(); + const auto& tuple_descs = parent.row_desc().tuple_descriptors(); for (auto tuple_desc : tuple_descs) { for (auto slot_desc : tuple_desc->slots()) { if (cache_param.output_slot_mapping.find(slot_desc->id()) != @@ -53,8 +53,6 @@ Status CacheSourceLocalState::init(RuntimeState* state, LocalStateInfo& info) { } } - // 2. build cache key by digest_tablet_id - RETURN_IF_ERROR(QueryCache::build_cache_key(scan_ranges, cache_param, &_cache_key, &_version)); std::vector cache_tablet_ids; cache_tablet_ids.reserve(scan_ranges.size()); for (const auto& scan_range : scan_ranges) { @@ -70,14 +68,38 @@ Status CacheSourceLocalState::init(RuntimeState* state, LocalStateInfo& info) { } custom_profile()->add_info_string("CacheTabletId", tablet_ids_str); - // 3. lookup the cache and find proper slot order - if (!cache_param.force_refresh_query_cache) { - hit_cache = _global_cache->lookup(_cache_key, _version, &_query_cache_handle); + // 2. consume the per-instance cache decision. It is made exactly once for + // this instance and shared with the olap scan operator, so both operators + // always act consistently (e.g. the scan skips scanning if and only if this + // operator emits the cached blocks) -- whatever their init order is, and + // even if the entry gets evicted in between (the decision pins it). + if (parent._query_cache_runtime == nullptr) { + // Should not happen: the fragment context always creates the runtime + // together with this operator. Degrade to an uncached scan. + LOG(WARNING) << "query cache runtime is absent, node id " << cache_param.node_id; + _need_insert_cache = false; + custom_profile()->add_info_string("HitCache", "0"); + return Status::OK(); } - custom_profile()->add_info_string("HitCache", std::to_string(hit_cache)); - if (hit_cache) { - _hit_cache_results = _query_cache_handle.get_cache_result(); - auto hit_cache_slot_orders = _query_cache_handle.get_cache_slot_orders(); + _global_cache = parent._query_cache_runtime->cache(); + _cache_decision = parent._query_cache_runtime->get_or_make_decision(scan_ranges); + _cache_key = _cache_decision->cache_key; + _version = _cache_decision->current_version; + + using Mode = QueryCacheInstanceDecision::Mode; + const bool hit_cache = _cache_decision->mode == Mode::HIT; + _is_incremental = _cache_decision->mode == Mode::INCREMENTAL; + // HIT emits the entry unchanged, so there is nothing to write back; both + // MISS and INCREMENTAL rebuild the entry (from scratch / by merge), unless + // the decision already knows the merged entry could never fit the entry + // limits (write_back_feasible == false). + _need_insert_cache = + _cache_decision->key_valid && !hit_cache && _cache_decision->write_back_feasible; + _insert_delta_count = _is_incremental ? _cache_decision->cached_delta_count + 1 : 0; + + if (hit_cache || _is_incremental) { + _hit_cache_results = _cache_decision->handle.get_cache_result(); + auto hit_cache_slot_orders = _cache_decision->handle.get_cache_slot_orders(); if (_slot_orders != *hit_cache_slot_orders) { for (auto slot_id : _slot_orders) { @@ -96,6 +118,18 @@ Status CacheSourceLocalState::init(RuntimeState* state, LocalStateInfo& info) { } } + custom_profile()->add_info_string("HitCache", std::to_string(hit_cache)); + custom_profile()->add_info_string("HitCacheStale", std::to_string(_is_incremental)); + if (_is_incremental) { + custom_profile()->add_info_string( + "IncrementalDeltaVersions", + fmt::format("({}, {}]", _cache_decision->cached_version, _version)); + } + if (!_cache_decision->incremental_fallback_reason.empty()) { + custom_profile()->add_info_string("IncrementalFallbackReason", + _cache_decision->incremental_fallback_reason); + } + return Status::OK(); } @@ -107,6 +141,32 @@ Status CacheSourceLocalState::open(RuntimeState* state) { return Status::OK(); } +bool CacheSourceLocalState::_account_write_back(int64_t rows, int64_t bytes) { + _current_query_cache_rows += rows; + _current_query_cache_bytes += bytes; + const auto& cache_param = _parent->cast()._cache_param; + if (cache_param.entry_max_bytes < _current_query_cache_bytes || + cache_param.entry_max_rows < _current_query_cache_rows) { + // over the max bytes/rows: pass the data through, no need to do cache + _local_cache_blocks.clear(); + _need_insert_cache = false; + return false; + } + return true; +} + +Status CacheSourceLocalState::_append_block_for_write_back(Block& block) { + if (!_account_write_back(block.rows(), block.allocated_bytes())) { + return Status::OK(); + } + auto& cloned = _local_cache_blocks.emplace_back(Block::create_unique()); + cloned->swap(block.clone_empty()); + ScopedMutableBlock scoped_mutable_block(cloned.get()); + auto& mutable_block = scoped_mutable_block.mutable_block(); + RETURN_IF_ERROR(mutable_block.merge(block)); + return Status::OK(); +} + std::string CacheSourceLocalState::debug_string(int indentation_level) const { fmt::memory_buffer debug_string_buffer; fmt::format_to(debug_string_buffer, "{}", Base::debug_string(indentation_level)); @@ -125,7 +185,49 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b block->clear_column_data(_row_descriptor.num_materialized_slots()); bool need_clone_empty = block->columns() == 0; - if (local_state._hit_cache_results == nullptr) { + const bool has_cached_block = + local_state._hit_cache_results != nullptr && + local_state._hit_cache_pos < local_state._hit_cache_results->size(); + + if (has_cached_block) { + // Emit one cached block: the whole result on HIT, or the cached part + // ahead of the delta on INCREMENTAL. Both the cached blocks and the + // delta blocks are partial aggregation states, so the upstream merge + // aggregation combines them into the final result. + // Note: this operator is only scheduled once the data queue has data or + // finished, so on INCREMENTAL the cached blocks are not emitted before + // the first delta block arrives. Making the source dependency initially + // ready for HIT/INCREMENTAL would overlap emitting the cached part with + // the delta scan -- a possible future latency optimization. + const auto& hit_cache_block = + local_state._hit_cache_results->at(local_state._hit_cache_pos++); + if (need_clone_empty) { + *block = hit_cache_block->clone_empty(); + } + { + ScopedMutableBlock scoped_mutable_block(block); + auto& mutable_block = scoped_mutable_block.mutable_block(); + RETURN_IF_ERROR(mutable_block.merge(*hit_cache_block)); + scoped_mutable_block.restore(); + } + if (!local_state._hit_cache_column_orders.empty()) { + auto datas = block->get_columns_with_type_and_name(); + block->clear(); + for (auto loc : local_state._hit_cache_column_orders) { + block->insert(datas[loc]); + } + } + if (local_state._is_incremental && local_state._need_insert_cache) { + // The emitted block is already reordered to this query's slot + // orders; keep a copy so the written-back entry holds + // "cached + delta" under one consistent slot order. + RETURN_IF_ERROR(local_state._append_block_for_write_back(*block)); + } + } else if (local_state._hit_cache_results != nullptr && !local_state._is_incremental) { + // HIT: all cached blocks are emitted. + *eos = true; + } else { + // MISS, or the delta phase of INCREMENTAL after the cached blocks. Defer insert_cache([&] { if (*eos) { local_state.custom_profile()->add_info_string( @@ -134,7 +236,8 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b local_state._global_cache->insert(local_state._cache_key, local_state._version, local_state._local_cache_blocks, local_state._slot_orders, - local_state._current_query_cache_bytes); + local_state._current_query_cache_bytes, + local_state._insert_delta_count); local_state._local_cache_blocks.clear(); } } @@ -160,42 +263,13 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b auto& mutable_block = scoped_mutable_block.mutable_block(); RETURN_IF_ERROR(mutable_block.merge(*output_block)); scoped_mutable_block.restore(); - local_state._current_query_cache_rows += output_block->rows(); - auto mem_consume = output_block->allocated_bytes(); - local_state._current_query_cache_bytes += mem_consume; - - if (_cache_param.entry_max_bytes < local_state._current_query_cache_bytes || - _cache_param.entry_max_rows < local_state._current_query_cache_rows) { - // over the max bytes, pass through the data, no need to do cache - local_state._local_cache_blocks.clear(); - local_state._need_insert_cache = false; - } else { + if (local_state._account_write_back(output_block->rows(), + output_block->allocated_bytes())) { local_state._local_cache_blocks.emplace_back(std::move(output_block)); } } else { *block = std::move(*output_block); } - } else { - if (local_state._hit_cache_pos < local_state._hit_cache_results->size()) { - const auto& hit_cache_block = - local_state._hit_cache_results->at(local_state._hit_cache_pos++); - if (need_clone_empty) { - *block = hit_cache_block->clone_empty(); - } - ScopedMutableBlock scoped_mutable_block(block); - auto& mutable_block = scoped_mutable_block.mutable_block(); - RETURN_IF_ERROR(mutable_block.merge(*hit_cache_block)); - scoped_mutable_block.restore(); - if (!local_state._hit_cache_column_orders.empty()) { - auto datas = block->get_columns_with_type_and_name(); - block->clear(); - for (auto loc : local_state._hit_cache_column_orders) { - block->insert(datas[loc]); - } - } - } else { - *eos = true; - } } local_state.reached_limit(block, eos); diff --git a/be/src/exec/operator/cache_source_operator.h b/be/src/exec/operator/cache_source_operator.h index 3a68bd337fc5b7..1d9da4ae183b18 100644 --- a/be/src/exec/operator/cache_source_operator.h +++ b/be/src/exec/operator/cache_source_operator.h @@ -48,6 +48,16 @@ class CacheSourceLocalState final : public PipelineXLocalState; + // Account `rows`/`bytes` against the entry limits. Once the limits are + // exceeded, drop the pending write-back blocks and stop caching (the data + // itself still passes through to the parent). Returns whether the entry is + // still cacheable. + bool _account_write_back(int64_t rows, int64_t bytes); + // Clone `block` (already reordered to this query's slot orders) into the + // write-back set. Used in INCREMENTAL mode for the cached blocks, so the + // written-back entry holds "cached + delta" under one consistent order. + Status _append_block_for_write_back(Block& block); + QueryCache* _global_cache = QueryCache::instance(); std::string _cache_key {}; @@ -58,7 +68,18 @@ class CacheSourceLocalState final : public PipelineXLocalState _cache_decision; + // Shortcut for _cache_decision->mode == INCREMENTAL: after the cached + // blocks are emitted, the delta partial result is drained from the data + // queue, and both are written back as the merged entry. + bool _is_incremental = false; + // delta_count to write back: 0 for a full recompute, cached + 1 for an + // incremental merge (drives periodic compaction, see QueryCacheRuntime). + int64_t _insert_delta_count = 0; + std::vector* _hit_cache_results = nullptr; std::vector _hit_cache_column_orders; int _hit_cache_pos = 0; @@ -68,8 +89,11 @@ class CacheSourceOperatorX final : public OperatorX { public: using Base = OperatorX; CacheSourceOperatorX(ObjectPool* pool, int plan_node_id, int operator_id, - const TQueryCacheParam& cache_param) - : Base(pool, plan_node_id, operator_id), _cache_param(cache_param) { + const TQueryCacheParam& cache_param, + std::shared_ptr query_cache_runtime) + : Base(pool, plan_node_id, operator_id), + _cache_param(cache_param), + _query_cache_runtime(std::move(query_cache_runtime)) { _op_name = "CACHE_SOURCE_OPERATOR"; }; @@ -90,6 +114,7 @@ class CacheSourceOperatorX final : public OperatorX { private: TQueryCacheParam _cache_param; + std::shared_ptr _query_cache_runtime; bool _has_data(RuntimeState* state) const { auto& local_state = get_local_state(state); return local_state._shared_state->data_queue.remaining_has_data(); diff --git a/be/src/exec/operator/olap_scan_operator.cpp b/be/src/exec/operator/olap_scan_operator.cpp index a5ea092ecc0bf7..7a927d1de63b1c 100644 --- a/be/src/exec/operator/olap_scan_operator.cpp +++ b/be/src/exec/operator/olap_scan_operator.cpp @@ -698,6 +698,13 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { bool read_row_binlog = p._olap_scan_node.__isset.read_row_binlog && p._olap_scan_node.read_row_binlog; bool has_tso_predicate = _scan_ranges[0]->__isset.start_tso || _scan_ranges[0]->__isset.end_tso; + // Query cache incremental merge: the read sources only cover the delta + // versions (cached_version, current_version], see prepare(). + const bool cache_incremental = + _query_cache_decision != nullptr && + _query_cache_decision->mode == QueryCacheInstanceDecision::Mode::INCREMENTAL; + const int64_t cache_start_version = + cache_incremental ? _query_cache_decision->cached_version + 1 : 0; // The flag of preagg's meaning is whether return pre agg data(or partial agg data) // PreAgg ON: The storage layer returns partially aggregated data without additional processing. (Fast data reading) @@ -705,7 +712,10 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { // And the user send a query like select userid,count(*) from base table group by userid. // then the storage layer do not need do aggregation, it could just return the partial agg data, because the compute layer will do aggregation. // PreAgg OFF: The storage layer must complete pre-aggregation and return fully aggregated data. (Slow data reading) - if (enable_parallel_scan && !p._should_run_serial && + // Incremental merge deltas are small by construction, so the parallel + // scanner builder (which redistributes rowsets by size) is pointless for + // them and would lose the delta version range; use plain scanners instead. + if (enable_parallel_scan && !cache_incremental && !p._should_run_serial && p._push_down_agg_type == TPushAggOp::NONE && (_storage_no_merge() || p._olap_scan_node.is_preaggregation) // binlog need to be read in order @@ -817,6 +827,7 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { palo_scan_range.__isset.end_tso ? std::make_optional(palo_scan_range.end_tso) : std::nullopt, + cache_start_version, }); RETURN_IF_ERROR(scanner->init(state(), _conjuncts)); scanners->push_back(std::move(scanner)); @@ -981,7 +992,31 @@ Status OlapScanLocalState::prepare(RuntimeState* state) { } } + const bool cache_incremental = + _query_cache_decision != nullptr && + _query_cache_decision->mode == QueryCacheInstanceDecision::Mode::INCREMENTAL; for (size_t i = 0; i < _scan_ranges.size(); i++) { + if (cache_incremental) { + // Scan only the delta rowsets in (cached_version, current_version]. + // The read source was already captured (and verified to contain no + // delete predicate) when the cache decision was made, so a capture + // failure cannot surface here, where the cache source operator has + // already committed to emitting the cached blocks. take() returning + // null would mean the FE invariant "one instance per tablet set, no + // shared scan under query cache" was broken (someone else consumed + // this tablet's read source); failing fast is the only safe answer, + // because a silent full-scan fallback would emit the data twice + // (cached blocks + full scan). + auto delta_source = + _query_cache_decision->take_delta_read_source(_tablets[i].tablet->tablet_id()); + if (delta_source == nullptr) { + return Status::InternalError( + "query cache incremental read source is absent, tablet_id={}", + _tablets[i].tablet->tablet_id()); + } + _read_sources[i] = std::move(*delta_source); + continue; + } _read_sources[i] = DORIS_TRY(_tablets[i].tablet->capture_read_source( {0, _tablets[i].version}, {.skip_missing_versions = _state->skip_missing_version(), @@ -1049,28 +1084,29 @@ TOlapScanNode& OlapScanLocalState::olap_scan_node() const { void OlapScanLocalState::set_scan_ranges(RuntimeState* state, const std::vector& scan_ranges) { - const auto& cache_param = _parent->cast()._cache_param; - bool hit_cache = false; - // read binlog scan should not participate in query cache. - if (olap_scan_node().__isset.read_row_binlog && olap_scan_node().read_row_binlog) { - hit_cache = false; - } else if (!cache_param.digest.empty() && !cache_param.force_refresh_query_cache) { - std::string cache_key; - int64_t version = 0; - auto status = QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version); - if (!status.ok()) { - throw doris::Exception(doris::ErrorCode::INTERNAL_ERROR, status.msg()); + auto& parent = _parent->cast(); + // Consume the per-instance cache decision. It is made exactly once and + // shared with the cache source operator of this fragment, so the two + // operators always agree: the scan skips scanning if and only if the cache + // source emits the cached blocks (HIT), and scans only the delta rowsets if + // and only if the cache source merges them with the cached blocks + // (INCREMENTAL). The decision also pins the cache entry, so it cannot be + // evicted between the two operators' init. + // Note: an invalid cache key (e.g. FE could not align this instance to one + // partition, so tablets carry different versions) now degrades to an + // uncached full scan instead of failing the query. + if (parent._query_cache_runtime != nullptr) { + _query_cache_decision = parent._query_cache_runtime->get_or_make_decision(scan_ranges); + if (_query_cache_decision->mode == QueryCacheInstanceDecision::Mode::HIT) { + // Exact hit: the cache source emits the entry, nothing to scan. + return; } - doris::QueryCacheHandle handle; - hit_cache = QueryCache::instance()->lookup(cache_key, version, &handle); } - if (!hit_cache) { - for (auto& scan_range : scan_ranges) { - DCHECK(scan_range.scan_range.__isset.palo_scan_range); - _scan_ranges.emplace_back(new TPaloScanRange(scan_range.scan_range.palo_scan_range)); - COUNTER_UPDATE(_tablet_counter, 1); - } + for (auto& scan_range : scan_ranges) { + DCHECK(scan_range.scan_range.__isset.palo_scan_range); + _scan_ranges.emplace_back(new TPaloScanRange(scan_range.scan_range.palo_scan_range)); + COUNTER_UPDATE(_tablet_counter, 1); } } @@ -1263,10 +1299,12 @@ Status OlapScanLocalState::_build_key_ranges_and_filters() { OlapScanOperatorX::OlapScanOperatorX(ObjectPool* pool, const TPlanNode& tnode, int operator_id, const DescriptorTbl& descs, int parallel_tasks, - const TQueryCacheParam& param) + const TQueryCacheParam& param, + std::shared_ptr query_cache_runtime) : ScanOperatorX(pool, tnode, operator_id, descs, parallel_tasks), _olap_scan_node(tnode.olap_scan_node), - _cache_param(param) { + _cache_param(param), + _query_cache_runtime(std::move(query_cache_runtime)) { _output_tuple_id = tnode.olap_scan_node.tuple_id; if (_olap_scan_node.__isset.sort_info && _olap_scan_node.__isset.sort_limit) { DORIS_CHECK(_limit < 0); diff --git a/be/src/exec/operator/olap_scan_operator.h b/be/src/exec/operator/olap_scan_operator.h index 4e8bab717c6bfa..1d2d2015039d6a 100644 --- a/be/src/exec/operator/olap_scan_operator.h +++ b/be/src/exec/operator/olap_scan_operator.h @@ -33,6 +33,8 @@ namespace doris { class OlapScanner; +class QueryCacheRuntime; +struct QueryCacheInstanceDecision; } // namespace doris namespace doris { @@ -343,6 +345,12 @@ class OlapScanLocalState final : public ScanLocalState { std::vector _tablets; std::vector _read_sources; + // The per-instance query cache decision shared with the cache source + // operator of the same fragment. Null when the query cache is disabled. + // HIT: leave _scan_ranges empty so nothing is scanned; INCREMENTAL: scan + // only the pre-captured delta read sources in (cached, current] version. + std::shared_ptr _query_cache_decision; + std::map _slot_id_to_virtual_column_expr; // ---- Runtime-filter partition pruning ---- @@ -358,7 +366,8 @@ class OlapScanOperatorX final : public ScanOperatorX { public: OlapScanOperatorX(ObjectPool* pool, const TPlanNode& tnode, int operator_id, const DescriptorTbl& descs, int parallel_tasks, - const TQueryCacheParam& cache_param); + const TQueryCacheParam& cache_param, + std::shared_ptr query_cache_runtime = nullptr); Status prepare(RuntimeState* state) override; @@ -374,6 +383,10 @@ class OlapScanOperatorX final : public ScanOperatorX { friend class OlapScanLocalState; TOlapScanNode _olap_scan_node; TQueryCacheParam _cache_param; + // Shared with the cache source operator of the same fragment so both + // consume the same per-instance cache decision (see QueryCacheRuntime). + // Null when the query cache is disabled. + std::shared_ptr _query_cache_runtime; TabletSchemaSPtr _tablet_schema; }; diff --git a/be/src/exec/pipeline/pipeline_fragment_context.cpp b/be/src/exec/pipeline/pipeline_fragment_context.cpp index 5664042dd24e15..45ad8366180258 100644 --- a/be/src/exec/pipeline/pipeline_fragment_context.cpp +++ b/be/src/exec/pipeline/pipeline_fragment_context.cpp @@ -1519,9 +1519,24 @@ Status PipelineFragmentContext::_create_operator(ObjectPool* pool, const TPlanNo bool fe_with_old_version = false; switch (tnode.node_type) { case TPlanNodeType::OLAP_SCAN_NODE: { + std::shared_ptr query_cache_runtime; + if (enable_query_cache) { + if (_query_cache_runtime == nullptr) { + _query_cache_runtime = + std::make_shared(_params.fragment.query_cache_param); + } + if (tnode.olap_scan_node.__isset.read_row_binlog && + tnode.olap_scan_node.read_row_binlog) { + // Row-binlog scans read a different data stream: they must + // neither serve nor fill the query cache. + _query_cache_runtime->disable_for_binlog_scan(); + } + query_cache_runtime = _query_cache_runtime; + } op = std::make_shared( pool, tnode, next_operator_id(), descs, _num_instances, - enable_query_cache ? _params.fragment.query_cache_param : TQueryCacheParam {}); + enable_query_cache ? _params.fragment.query_cache_param : TQueryCacheParam {}, + std::move(query_cache_runtime)); RETURN_IF_ERROR(cur_pipe->add_operator(op, _parallel_instances)); fe_with_old_version = !tnode.__isset.is_serial_operator; break; @@ -1583,8 +1598,13 @@ Status PipelineFragmentContext::_create_operator(ObjectPool* pool, const TPlanNo auto create_query_cache_operator = [&](PipelinePtr& new_pipe) { auto cache_node_id = _params.local_params[0].per_node_scan_ranges.begin()->first; auto cache_source_id = next_operator_id(); + if (_query_cache_runtime == nullptr) { + _query_cache_runtime = + std::make_shared(_params.fragment.query_cache_param); + } op = std::make_shared(pool, cache_node_id, cache_source_id, - _params.fragment.query_cache_param); + _params.fragment.query_cache_param, + _query_cache_runtime); RETURN_IF_ERROR(cur_pipe->add_operator(op, _parallel_instances)); const auto downstream_pipeline_id = cur_pipe->id(); diff --git a/be/src/exec/pipeline/pipeline_fragment_context.h b/be/src/exec/pipeline/pipeline_fragment_context.h index fe1d2a6c065ba0..1a63426738bef8 100644 --- a/be/src/exec/pipeline/pipeline_fragment_context.h +++ b/be/src/exec/pipeline/pipeline_fragment_context.h @@ -48,6 +48,7 @@ class ExecEnv; class RuntimeFilterMergeControllerEntity; class TDataSink; class TPipelineFragmentParams; +class QueryCacheRuntime; class Dependency; struct LocalExchangeSharedState; @@ -379,6 +380,12 @@ class PipelineFragmentContext : public TaskExecutionContext { TPipelineFragmentParams _params; int32_t _parallel_instances = 0; + // Query cache context of this fragment, shared by the olap scan operator + // and the cache source operator so both consume the same per-instance + // cache decision (HIT / INCREMENTAL / MISS). Created lazily when the + // fragment carries a query_cache_param. See QueryCacheRuntime. + std::shared_ptr _query_cache_runtime; + std::atomic _need_notify_close = false; // Holds the brpc ClosureGuard for async wait-close during recursive CTE rerun. // When the PFC finishes closing and is destroyed, the shared_ptr destructor fires diff --git a/be/src/exec/scan/olap_scanner.cpp b/be/src/exec/scan/olap_scanner.cpp index a9ef277c373002..894e7253c16266 100644 --- a/be/src/exec/scan/olap_scanner.cpp +++ b/be/src/exec/scan/olap_scanner.cpp @@ -80,7 +80,7 @@ OlapScanner::OlapScanner(ScanLocalStateBase* parent, OlapScanner::Params&& param .reader_type = params.read_row_binlog ? ReaderType::READER_BINLOG : ReaderType::READER_QUERY, .aggregation = params.aggregation, - .version = {0, params.version}, + .version = {params.start_version, params.version}, .start_key {}, .end_key {}, .predicates {}, diff --git a/be/src/exec/scan/olap_scanner.h b/be/src/exec/scan/olap_scanner.h index eb8f8dee4795bf..268165704ff770 100644 --- a/be/src/exec/scan/olap_scanner.h +++ b/be/src/exec/scan/olap_scanner.h @@ -74,6 +74,10 @@ class OlapScanner : public Scanner { TBinlogScanType::type binlog_scan_type = TBinlogScanType::NONE; std::optional start_tso; std::optional end_tso; + // Non-zero only for query cache incremental merge: read the delta + // rowsets in [start_version, version] instead of the full snapshot + // [0, version]. The read_source is pre-captured accordingly. + int64_t start_version = 0; }; OlapScanner(ScanLocalStateBase* parent, Params&& params); diff --git a/be/src/runtime/query_cache/query_cache.cpp b/be/src/runtime/query_cache/query_cache.cpp index 28610d44808686..98112f3854f48b 100644 --- a/be/src/runtime/query_cache/query_cache.cpp +++ b/be/src/runtime/query_cache/query_cache.cpp @@ -17,7 +17,14 @@ #include "runtime/query_cache/query_cache.h" +#include "cloud/config.h" #include "common/logging.h" +#include "common/metrics/doris_metrics.h" +#include "storage/olap_common.h" +#include "storage/rowset/rowset.h" +#include "storage/rowset/rowset_reader.h" +#include "storage/tablet/base_tablet.h" +#include "storage/tablet/tablet_meta.h" namespace doris { @@ -39,8 +46,27 @@ int64_t QueryCacheHandle::get_cache_version() { return ((QueryCache::CacheValue*)(result_ptr))->version; } +int64_t QueryCacheHandle::get_cache_delta_count() { + DCHECK(_handle); + auto result_ptr = reinterpret_cast(_handle)->value; + return ((QueryCache::CacheValue*)(result_ptr))->delta_count; +} + +int64_t QueryCacheHandle::get_cache_total_bytes() { + DCHECK(_handle); + auto result_ptr = reinterpret_cast(_handle)->value; + return ((QueryCache::CacheValue*)(result_ptr))->total_bytes; +} + +int64_t QueryCacheHandle::get_cache_total_rows() { + DCHECK(_handle); + auto result_ptr = reinterpret_cast(_handle)->value; + return ((QueryCache::CacheValue*)(result_ptr))->total_rows; +} + void QueryCache::insert(const CacheKey& key, int64_t version, CacheResult& res, - const std::vector& slot_orders, int64_t cache_size) { + const std::vector& slot_orders, int64_t cache_size, + int64_t delta_count) { SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->query_cache_mem_tracker()); CacheResult cache_result; for (auto& block_data : res) { @@ -50,11 +76,12 @@ void QueryCache::insert(const CacheKey& key, int64_t version, CacheResult& res, auto st = mutable_block.merge(*block_data); DORIS_CHECK(st.ok()); } - auto cache_value_ptr = - std::make_unique(version, std::move(cache_result), slot_orders); + auto cache_value_ptr = std::make_unique( + version, std::move(cache_result), slot_orders, delta_count, cache_size); QueryCacheHandle(this, LRUCachePolicy::insert(key, (void*)cache_value_ptr.release(), cache_size, cache_size, CachePriority::NORMAL)); + DorisMetrics::instance()->query_cache_write_back_total->increment(1); } bool QueryCache::lookup(const CacheKey& key, int64_t version, doris::QueryCacheHandle* handle) { @@ -70,4 +97,267 @@ bool QueryCache::lookup(const CacheKey& key, int64_t version, doris::QueryCacheH return false; } +bool QueryCache::lookup_any_version(const CacheKey& key, QueryCacheHandle* handle) { + SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->query_cache_mem_tracker()); + auto* lru_handle = LRUCachePolicy::lookup(key); + if (lru_handle) { + *handle = QueryCacheHandle(this, lru_handle); + return true; + } + return false; +} + +QueryCacheInstanceDecision::~QueryCacheInstanceDecision() = default; + +std::unique_ptr QueryCacheInstanceDecision::take_delta_read_source( + int64_t tablet_id) { + std::lock_guard lock(_take_lock); + auto it = _delta_read_sources.find(tablet_id); + if (it == _delta_read_sources.end()) { + return nullptr; + } + auto res = std::move(it->second); + _delta_read_sources.erase(it); + return res; +} + +std::shared_ptr QueryCacheRuntime::get_or_make_decision( + const std::vector& scan_ranges) { + std::string cache_key; + int64_t version = 0; + Status st = QueryCache::build_cache_key(scan_ranges, _param, &cache_key, &version); + if (!st.ok()) { + // No reliable cache key for this instance (e.g. FE could not align the + // instance to a single partition so tablets carry different versions). + // Degrade to an uncached scan instead of failing the query: both the + // scan operator and the cache source operator observe the shared MISS + // decision with key_valid=false, so nothing is looked up and nothing + // is written back. This is an expected plan shape, so log only once + // per fragment instead of twice per instance. + std::lock_guard lock(_lock); + if (_invalid_decision == nullptr) { + LOG(WARNING) << "query cache degrades to uncached scan, node_id=" << _param.node_id + << ", reason: " << st.to_string(); + _invalid_decision = std::make_shared(); + } + return _invalid_decision; + } + + // The lock also serializes decision making of different instances of this + // fragment. That is acceptable in the common case: a decision only touches + // tablet metadata (no data IO), so it finishes in microseconds to + // milliseconds. On very wide fragments (hundreds of tablets per instance) + // combined with tablet header lock contention (e.g. compaction meta + // commits) this serialization can stretch the fragment init tail; if that + // ever shows up in profiles, move the capture out of the lock and let the + // losing racer drop its duplicate decision. + std::lock_guard lock(_lock); + auto it = _decisions.find(cache_key); + if (it != _decisions.end()) { + return it->second; + } + + auto decision = std::make_shared(); + decision->key_valid = true; + decision->cache_key = cache_key; + decision->current_version = version; + _make_decision(scan_ranges, decision.get()); + _decisions.emplace(std::move(cache_key), decision); + return decision; +} + +void QueryCacheRuntime::_make_decision(const std::vector& scan_ranges, + QueryCacheInstanceDecision* decision) { + decision->mode = QueryCacheInstanceDecision::Mode::MISS; + if (_binlog_scan) { + // A row-binlog scan reads a different data stream under the same plan + // digest: it must neither serve cached blocks nor write its output + // back (that would poison the cache for normal queries). + decision->key_valid = false; + return; + } + if (_param.force_refresh_query_cache) { + return; + } + + QueryCacheHandle handle; + if (!_cache->lookup_any_version(decision->cache_key, &handle)) { + return; + } + // Pin the entry: as long as this decision object lives, the entry cannot be + // evicted, so every operator consuming this decision sees the same blocks. + decision->handle = std::move(handle); + + if (decision->handle.get_cache_version() == decision->current_version) { + decision->mode = QueryCacheInstanceDecision::Mode::HIT; + decision->cached_delta_count = decision->handle.get_cache_delta_count(); + return; + } + + if (_try_prepare_incremental(scan_ranges, decision)) { + decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; + DorisMetrics::instance()->query_cache_stale_hit_total->increment(1); + return; + } + + // Stale entry not reusable: drop the pin and fall back to a full scan. + decision->_delta_read_sources.clear(); + decision->handle = QueryCacheHandle(); + decision->mode = QueryCacheInstanceDecision::Mode::MISS; + if (!decision->incremental_fallback_reason.empty()) { + // Only count real fallbacks: an empty reason means incremental merge + // was not enabled for this query in the first place. + DorisMetrics::instance()->query_cache_incremental_fallback_total->increment(1); + } +} + +bool QueryCacheRuntime::_try_prepare_incremental(const std::vector& scan_ranges, + QueryCacheInstanceDecision* decision) { + if (!(_param.__isset.allow_incremental && _param.allow_incremental)) { + // Not a fallback: incremental merge is simply not enabled for this + // query, so leave incremental_fallback_reason empty. + return false; + } + // Cloud tablets capture rowsets through a different (partly asynchronous) + // path; incremental merge only supports local storage for now. + if (config::is_cloud_mode()) { + decision->incremental_fallback_reason = "cloud mode"; + return false; + } + + int64_t cached_version = decision->handle.get_cache_version(); + if (cached_version >= decision->current_version) { + // The entry is newer than what this replica is asked to read (e.g. the + // entry was filled from another replica with a newer visible version). + // A full scan of the requested version is the only safe answer. + decision->incremental_fallback_reason = "cached entry is newer"; + return false; + } + int64_t cached_delta_count = decision->handle.get_cache_delta_count(); + if (cached_delta_count >= config::query_cache_max_incremental_merge_count) { + // Force a full recompute to compact the entry: every incremental merge + // appends the delta blocks to the entry, so both the entry and the + // upstream merge get more fragmented as deltas accumulate. + decision->incremental_fallback_reason = "delta count reached compaction threshold"; + return false; + } + + for (const auto& scan_range : scan_ranges) { + int64_t tablet_id = scan_range.scan_range.palo_scan_range.tablet_id; + if (!_capture_tablet_delta(tablet_id, cached_version, decision)) { + return false; + } + } + + // If the cached entry alone already exceeds the entry limits, the merged + // entry (which can only be larger) could never be written back. Still scan + // only the delta, but tell the cache source upfront so it does not clone + // blocks for a write back that would be discarded anyway. Such an entry + // stays stale until compaction merges its delta away (then the capture + // above fails and a full recompute takes over). + if (decision->handle.get_cache_total_bytes() > _param.entry_max_bytes || + decision->handle.get_cache_total_rows() > _param.entry_max_rows) { + decision->write_back_feasible = false; + } + + decision->cached_version = cached_version; + decision->cached_delta_count = cached_delta_count; + return true; +} + +bool QueryCacheRuntime::_capture_tablet_delta(int64_t tablet_id, int64_t cached_version, + QueryCacheInstanceDecision* decision) { + auto tablet_res = ExecEnv::get_tablet(tablet_id); + if (!tablet_res) { + decision->incremental_fallback_reason = "tablet not found"; + return false; + } + BaseTabletSPtr tablet = std::move(tablet_res.value()); + // Defensive re-check of what FE promised with allow_incremental: + // "cached snapshot + delta rowsets == new snapshot" holds unconditionally + // for append-only DUP_KEYS data, and for merge-on-write UNIQUE data as + // long as the delta did not rewrite any row that predates the cached + // version (verified below once the delta rowsets are known). It never + // holds for merge-on-read UNIQUE data (duplicates are resolved by merging + // across rowsets at read time, so a delta-only scan cannot stand alone) + // nor for AGG tables (rows merge in the storage layer likewise). + const bool merge_on_write = tablet->keys_type() == KeysType::UNIQUE_KEYS && + tablet->enable_unique_key_merge_on_write(); + if (tablet->keys_type() != KeysType::DUP_KEYS && !merge_on_write) { + decision->incremental_fallback_reason = "keys type not append-only"; + return false; + } + + // quiet: on a version-graph miss (the delta merged away by compaction, an + // expected per-query situation) this option makes the capture API neither + // log nor error; it returns an EMPTY read source instead, so emptiness is + // checked below as the failure signal. + auto source_res = tablet->capture_read_source({cached_version + 1, decision->current_version}, + {.quiet = true}); + if (!source_res) { + // Non-pathfinding failures (e.g. a rowset object missing for a found + // path) still surface as errors even under quiet. + decision->incremental_fallback_reason = "delta versions not capturable"; + return false; + } + auto read_source = std::make_unique(std::move(source_res.value())); + if (read_source->rs_splits.empty()) { + // A non-empty version window always lies on a consistent path with at + // least one rowset; an empty capture therefore means the delta + // versions are gone, never "no new data". Treating it as INCREMENTAL + // would silently drop the delta rows from the result. + decision->incremental_fallback_reason = "delta versions not capturable"; + return false; + } + read_source->fill_delete_predicates(); + if (!read_source->delete_predicates.empty()) { + // A delete in the delta logically removes rows that are already folded + // into the cached partial result; that cannot be undone by merging, so + // fall back to a full recompute. + decision->incremental_fallback_reason = "delta contains delete predicates"; + return false; + } + if (merge_on_write && + _delta_rewrites_history(*tablet, *read_source, cached_version, decision->current_version)) { + // A load in the delta window marked rows of the cached snapshot as + // deleted through the delete bitmap (an upsert, a partial update or a + // delete sign hit a pre-existing key). Those rows are already folded + // into the cached partial result and cannot be subtracted, so only a + // full recompute is safe. The full run re-bases the entry at the new + // version, so an occasional backfill costs exactly one full scan and + // the next pure-append hour is incremental again. + decision->incremental_fallback_reason = "delta rewrites history rows"; + return false; + } + decision->_delta_read_sources[tablet_id] = std::move(read_source); + return true; +} + +bool QueryCacheRuntime::_delta_rewrites_history(BaseTablet& tablet, + const TabletReadSource& delta_source, + int64_t cached_version, int64_t current_version) { + RowsetIdUnorderedSet delta_rowsets; + for (const auto& split : delta_source.rs_splits) { + delta_rowsets.insert(split.rs_reader->rowset()->rowset_id()); + } + // The tablet delete bitmap is shared with concurrent loads, but entries + // stamped with a version <= current_version are immutable once that + // version is visible (merge-on-write publishes the bitmap update before + // the version becomes visible), so this scan is race free for the window + // we care about. Pending entries use DeleteBitmap::TEMP_VERSION_COMMON + // (= 0) and stay below the window as well. + const DeleteBitmap& delete_bitmap = tablet.tablet_meta()->delete_bitmap(); + std::shared_lock rdlock(delete_bitmap.lock); + for (const auto& [bitmap_key, bitmap] : delete_bitmap.delete_bitmap) { + const auto version = static_cast(std::get<2>(bitmap_key)); + if (version <= cached_version || version > current_version || bitmap.isEmpty()) { + continue; + } + if (!delta_rowsets.contains(std::get<0>(bitmap_key))) { + return true; + } + } + return false; +} + } // namespace doris diff --git a/be/src/runtime/query_cache/query_cache.h b/be/src/runtime/query_cache/query_cache.h index bfa90b011e34f8..f727159eac2915 100644 --- a/be/src/runtime/query_cache/query_cache.h +++ b/be/src/runtime/query_cache/query_cache.h @@ -23,9 +23,13 @@ #include #include +#include #include +#include #include #include +#include +#include #include "common/config.h" #include "common/status.h" @@ -41,6 +45,9 @@ namespace doris { +class BaseTablet; +struct TabletReadSource; + using CacheResult = std::vector; // A handle for mid-result from query lru cache. // The handle will automatically release the cache entry when it is destroyed. @@ -73,12 +80,22 @@ class QueryCacheHandle { return *this; } + bool valid() const { return _handle != nullptr; } + std::vector* get_cache_slot_orders(); CacheResult* get_cache_result(); int64_t get_cache_version(); + // How many incremental merges have been accumulated on this entry since the + // last full recompute. See QueryCacheRuntime for the compaction policy. + int64_t get_cache_delta_count(); + + int64_t get_cache_total_bytes(); + + int64_t get_cache_total_rows(); + private: LRUCachePolicy* _cache = nullptr; Cache::Handle* _handle = nullptr; @@ -95,9 +112,28 @@ class QueryCache : public LRUCachePolicy { int64_t version; CacheResult result; std::vector slot_orders; - - CacheValue(int64_t v, CacheResult&& r, const std::vector& so) - : LRUCacheValueBase(), version(v), result(std::move(r)), slot_orders(so) {} + // Number of incremental merges accumulated on this entry since the last + // full recompute. 0 means the entry was produced by a full scan. + int64_t delta_count; + // Size of this entry, used to decide upfront whether an incremental + // merge could ever be written back under the entry_max_bytes/rows + // limits (a merged entry can only be larger than the cached one). + int64_t total_bytes; + int64_t total_rows; + + CacheValue(int64_t v, CacheResult&& r, const std::vector& so, int64_t dc = 0, + int64_t bytes = 0) + : LRUCacheValueBase(), + version(v), + result(std::move(r)), + slot_orders(so), + delta_count(dc), + total_bytes(bytes) { + total_rows = 0; + for (const auto& block : result) { + total_rows += block->rows(); + } + } }; // Create global instance of this class @@ -213,7 +249,155 @@ class QueryCache : public LRUCachePolicy { bool lookup(const CacheKey& key, int64_t version, QueryCacheHandle* handle); + // Look up the entry by key regardless of its version. The caller decides + // whether the entry is an exact hit (cached version == expected version) or + // a stale entry usable for incremental merge. Returns false if the key is + // not in the cache at all. + bool lookup_any_version(const CacheKey& key, QueryCacheHandle* handle); + void insert(const CacheKey& key, int64_t version, CacheResult& result, - const std::vector& solt_orders, int64_t cache_size); + const std::vector& solt_orders, int64_t cache_size, int64_t delta_count = 0); +}; + +// The per-fragment-instance decision of how the query cache participates in the +// execution, made exactly once (see QueryCacheRuntime) and consumed by both the +// olap scan operator and the cache source operator, so the two operators can +// never disagree (e.g. scan skips scanning because the entry looked fresh while +// cache source misses because the entry got evicted in between -- which would +// silently produce an empty result and poison the cache with it). +struct QueryCacheInstanceDecision { + enum class Mode { + // Run the full scan and (if the key is valid) write the result back. + MISS, + // The cached entry matches the current version: emit cached blocks, + // skip scanning entirely, do not write back. + HIT, + // A stale entry is reusable: scan only the delta rowsets in + // (cached_version, current_version], emit the cached blocks and the + // delta partial result side by side (the upstream merge aggregation + // combines them), then write the merged entry back. + INCREMENTAL, + }; + + ~QueryCacheInstanceDecision(); + + // Take the pre-captured delta read source of one tablet. Returns nullptr if + // absent (already taken or never captured). Only meaningful in INCREMENTAL + // mode; each tablet's read source can be consumed exactly once. + std::unique_ptr take_delta_read_source(int64_t tablet_id); + + Mode mode = Mode::MISS; + // False when build_cache_key failed (e.g. tablets in this instance carry + // different versions because FE could not align instances to partitions). + // In that case the query degrades to an uncached scan: no lookup, no write + // back, but the query itself still succeeds. + bool key_valid = false; + // False when the merged entry could never satisfy entry_max_bytes/rows + // because the reused cached entry alone already exceeds them: the query + // still scans only the delta (INCREMENTAL), but skips cloning blocks for a + // write back that would be discarded anyway. + bool write_back_feasible = true; + // Why a stale entry was not reused incrementally (empty when it was, or + // when incremental merge is not enabled for this query). For the query + // profile only. + std::string incremental_fallback_reason; + std::string cache_key; + // The version this query is reading (from the scan ranges). + int64_t current_version = 0; + // Only set in INCREMENTAL mode: the version of the reused stale entry. + int64_t cached_version = 0; + // Only set in HIT/INCREMENTAL mode: delta merges accumulated on the entry. + int64_t cached_delta_count = 0; + // Pins the cache entry in HIT/INCREMENTAL mode so it cannot be evicted (and + // its blocks cannot be freed) while this query is using it. Note the pin + // lives until the fragment is torn down; when the merged entry replaces + // this one under the same key, both stay in memory for that window and the + // LRU usage accounting only sees the new one (the mem tracker still sees + // both) -- bounded by (in-flight incremental queries) x entry size. + QueryCacheHandle handle; + +private: + friend class QueryCacheRuntime; + std::mutex _take_lock; + // INCREMENTAL mode: read sources of (cached_version, current_version] + // captured at decision time, keyed by tablet id. Captured eagerly so that a + // capture failure (e.g. the delta versions were merged away by compaction) + // downgrades the decision to MISS *before* any operator acts on it; if the + // scan discovered the failure only at prepare time, the cache source might + // already have decided to emit the stale blocks. + std::unordered_map> _delta_read_sources; }; + +// Fragment-level query cache context shared by the olap scan operator and the +// cache source operator of the same fragment. Both operators obtain the cache +// decision of their instance through get_or_make_decision(); the first caller +// makes the decision and the other one observes the same object, whatever the +// operator local-state init order is. +class QueryCacheRuntime { +public: + // `cache` is injectable for tests; production callers pass nullptr and the + // global instance is used. + explicit QueryCacheRuntime(const TQueryCacheParam& param, QueryCache* cache = nullptr) + : _param(param), _cache(cache != nullptr ? cache : QueryCache::instance()) {} + + QueryCache* cache() const { return _cache; } + + // Row-binlog scans read a different data stream and must not serve or fill + // the query cache. Called while building the operator tree (single + // threaded, before any local state init), so no locking is needed. + void disable_for_binlog_scan() { _binlog_scan = true; } + + // Idempotent: the first call for a given instance (identified by the cache + // key derived from its scan ranges) makes the decision, later calls return + // the same decision object. Never returns nullptr. + std::shared_ptr get_or_make_decision( + const std::vector& scan_ranges); + +#ifdef BE_TEST + // Tests inject a hand-crafted decision (e.g. INCREMENTAL) for an instance, + // since a real storage engine is unavailable to capture delta read sources. + void inject_decision_for_test(const std::string& cache_key, + std::shared_ptr decision) { + std::lock_guard lock(_lock); + _decisions[cache_key] = std::move(decision); + } +#endif + +private: + void _make_decision(const std::vector& scan_ranges, + QueryCacheInstanceDecision* decision); + + // Try to turn a stale entry into an INCREMENTAL decision. Returns true on + // success; on any failure the caller keeps the decision as MISS (full + // recompute), which is always safe. + bool _try_prepare_incremental(const std::vector& scan_ranges, + QueryCacheInstanceDecision* decision); + + // Validate one tablet for incremental merge and capture its delta read + // source of (cached_version, current_version]. On any failure records the + // fallback reason in the decision and returns false. + bool _capture_tablet_delta(int64_t tablet_id, int64_t cached_version, + QueryCacheInstanceDecision* decision); + + // Merge-on-write only: true if any delete-bitmap entry stamped with a + // version inside (cached_version, current_version] targets a rowset + // OUTSIDE the captured delta set, i.e. the delta window rewrote rows that + // are already folded into the cached partial result (an upsert, a partial + // update or a delete sign hit a key that predates the cached version). + // Entries targeting the delta rowsets themselves are harmless: the delta + // scan reads those rowsets with the delete bitmap applied. + static bool _delta_rewrites_history(BaseTablet& tablet, const TabletReadSource& delta_source, + int64_t cached_version, int64_t current_version); + + TQueryCacheParam _param; + QueryCache* _cache = nullptr; + bool _binlog_scan = false; + + std::mutex _lock; + std::map> _decisions; + // Shared by every instance whose cache key cannot be built (see + // get_or_make_decision): one immutable MISS decision, one log line. + std::shared_ptr _invalid_decision; +}; + } // namespace doris diff --git a/be/test/exec/operator/query_cache_operator_test.cpp b/be/test/exec/operator/query_cache_operator_test.cpp index 91c73b99077247..306f624d84ba0c 100644 --- a/be/test/exec/operator/query_cache_operator_test.cpp +++ b/be/test/exec/operator/query_cache_operator_test.cpp @@ -65,11 +65,14 @@ struct QueryCacheOperatorTest : public ::testing::Test { scan_ranges.push_back(scan_range); } void TearDown() override { - // Must clear state before query_cache_uptr is destroyed, because - // local states inside `state` hold QueryCacheHandle which references - // the QueryCache. C++ destroys members in reverse declaration order, - // so state (declared before query_cache_uptr) would outlive the cache. + // Must release every QueryCacheHandle holder before query_cache_uptr is + // destroyed. C++ destroys members in reverse declaration order, so + // `state` (local states) and `source`/`sink` (whose QueryCacheRuntime + // owns the decisions that pin cache entries) would otherwise outlive + // the cache and release their handles into a destroyed QueryCache. state.reset(); + source.reset(); + sink.reset(); } void create_local_state() { shared_state = sink->create_shared_state(); @@ -134,10 +137,6 @@ struct QueryCacheOperatorTest : public ::testing::Test { TEST_F(QueryCacheOperatorTest, test_no_hit_cache1) { sink = std::make_unique(); - source = std::make_unique(); - EXPECT_TRUE(source->set_child(child_op)); - child_op->_mock_row_desc.reset( - new MockRowDescriptor {{std::make_shared()}, &pool}); TQueryCacheParam cache_param; cache_param.node_id = 0; cache_param.digest = "test_digest"; @@ -147,7 +146,14 @@ TEST_F(QueryCacheOperatorTest, test_no_hit_cache1) { cache_param.entry_max_bytes = 1024 * 1024; cache_param.entry_max_rows = 1000; - source->_cache_param = cache_param; + // Exercise the production constructor once; the other tests use the + // BE_TEST default constructor and assign the members directly. + source = std::make_unique( + &pool, /*plan_node_id=*/0, /*operator_id=*/0, cache_param, + std::make_shared(cache_param, query_cache)); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); create_local_state(); std::cout << query_cache->get_element_count() << std::endl; @@ -189,6 +195,7 @@ TEST_F(QueryCacheOperatorTest, test_no_hit_cache2) { cache_param.entry_max_rows = 3; source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); create_local_state(); std::cout << query_cache->get_element_count() << std::endl; @@ -241,6 +248,7 @@ TEST_F(QueryCacheOperatorTest, test_hit_cache) { } source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); create_local_state(); std::cout << query_cache->get_element_count() << std::endl; @@ -276,4 +284,412 @@ TEST_F(QueryCacheOperatorTest, test_hit_cache) { } } +TEST_F(QueryCacheOperatorTest, test_stale_full_recompute) { + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + + std::string cache_key; + { + // A stale entry (version 100 < scan version 114514) without + // allow_incremental: plain miss, full recompute, write back overwrites. + int64_t version = 0; + EXPECT_TRUE( + QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({100, 200}); + query_cache->insert(cache_key, 100, result, {0}, 1); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + create_local_state(); + + EXPECT_TRUE(source_local_state->_need_insert_cache); + EXPECT_FALSE(source_local_state->_is_incremental); + + { + auto block = ColumnHelper::create_block({1, 2, 3, 4, 5}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + { + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + EXPECT_EQ(block.rows(), 5); + EXPECT_TRUE(ColumnHelper::block_equal( + block, ColumnHelper::create_block({1, 2, 3, 4, 5}))); + } + + // The stale entry is overwritten by the recomputed one. + doris::QueryCacheHandle handle; + EXPECT_TRUE(query_cache->lookup(cache_key, 114514, &handle)); + EXPECT_EQ(handle.get_cache_delta_count(), 0); +} + +TEST_F(QueryCacheOperatorTest, test_incremental_merge) { + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + { + // The cached partial blocks of version 100, already merged once before. + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({1, 2, 3, 4, 5}); + query_cache->insert(cache_key, 100, result, {0}, 1, /*delta_count=*/1); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + // Hand-craft the INCREMENTAL decision (capturing a real delta read source + // needs a storage engine, which unit tests do not have). The scan side + // would scan only (100, 114514] and feed the delta through the sink. + { + auto decision = std::make_shared(); + decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; + decision->key_valid = true; + decision->cache_key = cache_key; + decision->current_version = version; + decision->cached_version = 100; + decision->cached_delta_count = 1; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &decision->handle)); + source->_query_cache_runtime->inject_decision_for_test(cache_key, decision); + } + create_local_state(); + + EXPECT_TRUE(source_local_state->_need_insert_cache); + EXPECT_TRUE(source_local_state->_is_incremental); + const std::string* stale = + source_local_state->custom_profile()->get_info_string("HitCacheStale"); + ASSERT_NE(stale, nullptr); + EXPECT_EQ(*stale, "1"); + const std::string* delta_versions = + source_local_state->custom_profile()->get_info_string("IncrementalDeltaVersions"); + ASSERT_NE(delta_versions, nullptr); + EXPECT_EQ(*delta_versions, "(100, 114514]"); + + { + // The delta partial result produced by scanning only (100, 114514]. + auto block = ColumnHelper::create_block({6, 7}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + { + // First the cached blocks are emitted ... + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 5); + EXPECT_TRUE(ColumnHelper::block_equal( + block, ColumnHelper::create_block({1, 2, 3, 4, 5}))); + } + { + // ... then the delta from the data queue; both are partial aggregation + // states merged by the upstream aggregation. + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + EXPECT_EQ(block.rows(), 2); + EXPECT_TRUE(ColumnHelper::block_equal(block, + ColumnHelper::create_block({6, 7}))); + } + + // The merged entry is written back under the new version with an increased + // delta count, holding both the cached and the delta blocks. + doris::QueryCacheHandle handle; + EXPECT_TRUE(query_cache->lookup(cache_key, 114514, &handle)); + EXPECT_EQ(handle.get_cache_delta_count(), 2); + int64_t total_rows = 0; + for (const auto& cached_block : *handle.get_cache_result()) { + total_rows += cached_block->rows(); + } + EXPECT_EQ(total_rows, 7); +} + +TEST_F(QueryCacheOperatorTest, test_incremental_over_entry_limit) { + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + // The cached part alone (5 rows) already exceeds the limit, so the merged + // entry must not be written back; the stale entry stays untouched. + cache_param.entry_max_rows = 3; + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + { + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({1, 2, 3, 4, 5}); + query_cache->insert(cache_key, 100, result, {0}, 1, /*delta_count=*/0); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + { + auto decision = std::make_shared(); + decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; + decision->key_valid = true; + decision->cache_key = cache_key; + decision->current_version = version; + decision->cached_version = 100; + decision->cached_delta_count = 0; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &decision->handle)); + source->_query_cache_runtime->inject_decision_for_test(cache_key, decision); + } + create_local_state(); + + EXPECT_TRUE(source_local_state->_need_insert_cache); + + { + auto block = ColumnHelper::create_block({6, 7}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + { + // Emitting the cached blocks overruns entry_max_rows: caching stops but + // the data still flows. + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 5); + EXPECT_FALSE(source_local_state->_need_insert_cache); + } + { + // The delta passes through unchanged. + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + EXPECT_EQ(block.rows(), 2); + EXPECT_TRUE(ColumnHelper::block_equal(block, + ColumnHelper::create_block({6, 7}))); + } + + // No write back happened: the stale entry still carries version 100. + doris::QueryCacheHandle handle; + EXPECT_FALSE(query_cache->lookup(cache_key, 114514, &handle)); + doris::QueryCacheHandle stale_handle; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &stale_handle)); + EXPECT_EQ(stale_handle.get_cache_version(), 100); +} + +TEST_F(QueryCacheOperatorTest, test_incremental_write_back_infeasible) { + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + { + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({1, 2, 3, 4, 5}); + query_cache->insert(cache_key, 100, result, {0}, 1); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + // The decision already knows the merged entry could never fit the limits: + // scan only the delta, but never clone blocks for a doomed write back. + { + auto decision = std::make_shared(); + decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; + decision->key_valid = true; + decision->write_back_feasible = false; + decision->cache_key = cache_key; + decision->current_version = version; + decision->cached_version = 100; + decision->cached_delta_count = 0; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &decision->handle)); + source->_query_cache_runtime->inject_decision_for_test(cache_key, decision); + } + create_local_state(); + + EXPECT_FALSE(source_local_state->_need_insert_cache); + EXPECT_TRUE(source_local_state->_is_incremental); + + { + auto block = ColumnHelper::create_block({6, 7}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + { + // Cached blocks flow through without being cloned for a write back. + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 5); + EXPECT_TRUE(source_local_state->_local_cache_blocks.empty()); + } + { + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + EXPECT_EQ(block.rows(), 2); + EXPECT_TRUE(source_local_state->_local_cache_blocks.empty()); + } + + // No write back: the stale entry still carries version 100. + doris::QueryCacheHandle handle; + EXPECT_FALSE(query_cache->lookup(cache_key, 114514, &handle)); + doris::QueryCacheHandle stale_handle; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &stale_handle)); + EXPECT_EQ(stale_handle.get_cache_version(), 100); +} + +TEST_F(QueryCacheOperatorTest, test_incremental_fallback_reason_in_profile) { + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + // Use a tablet id that exists nowhere so fetching the tablet fails even if + // another suite left a storage engine registered in this test process. + constexpr int64_t kMissingTabletId = 424242424242; + scan_ranges.clear(); + TScanRangeParams scan_range; + TPaloScanRange palo_scan_range; + palo_scan_range.__set_tablet_id(kMissingTabletId); + palo_scan_range.__set_version("114514"); + scan_range.scan_range.__set_palo_scan_range(palo_scan_range); + scan_ranges.push_back(scan_range); + + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({kMissingTabletId, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + { + // A stale entry with incremental allowed, but the tablet does not exist + // in this test process: the decision falls back to a full recompute and + // reports why in the query profile. + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({100, 200}); + query_cache->insert(cache_key, 100, result, {0}, 1); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + create_local_state(); + + EXPECT_FALSE(source_local_state->_is_incremental); + EXPECT_TRUE(source_local_state->_need_insert_cache); + const std::string* reason = + source_local_state->custom_profile()->get_info_string("IncrementalFallbackReason"); + ASSERT_NE(reason, nullptr); + EXPECT_EQ(*reason, "tablet not found"); +} + +TEST_F(QueryCacheOperatorTest, test_missing_runtime_degrades_to_uncached) { + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + + // No runtime injected: init degrades to an uncached scan, data passes + // through and nothing is written back. + source->_cache_param = cache_param; + create_local_state(); + + EXPECT_FALSE(source_local_state->_need_insert_cache); + + { + auto block = ColumnHelper::create_block({1, 2, 3}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + { + Block block; + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + EXPECT_EQ(block.rows(), 3); + EXPECT_TRUE(ColumnHelper::block_equal( + block, ColumnHelper::create_block({1, 2, 3}))); + } + EXPECT_EQ(query_cache->get_element_count(), 0); +} + } // namespace doris diff --git a/be/test/exec/pipeline/query_cache_test.cpp b/be/test/exec/pipeline/query_cache_test.cpp index ec0f8c6c697686..c6c0e08e81aad7 100644 --- a/be/test/exec/pipeline/query_cache_test.cpp +++ b/be/test/exec/pipeline/query_cache_test.cpp @@ -17,13 +17,33 @@ #include "runtime/query_cache/query_cache.h" +#include +#include +#include #include +#include #include +#include #include +#include "cloud/config.h" +#include "common/config.h" +#include "common/metrics/doris_metrics.h" #include "core/data_type/data_type_number.h" +#include "io/fs/local_file_system.h" +#include "json2pb/json_to_pb.h" +#include "storage/data_dir.h" +#include "storage/options.h" +#include "storage/rowset/rowset_meta.h" +#include "storage/storage_engine.h" +#include "storage/tablet/base_tablet.h" +#include "storage/tablet/tablet.h" +#include "storage/tablet/tablet_manager.h" +#include "storage/tablet/tablet_meta.h" #include "testutil/column_helper.h" +#include "util/debug_points.h" +#include "util/uid_util.h" namespace doris { class QueryCacheTest : public testing::Test { @@ -287,4 +307,542 @@ TEST_F(QueryCacheTest, insert_and_lookup) { // ./run-be-ut.sh --run --filter=DataQueueTest.* +namespace { + +std::vector make_scan_ranges(int64_t tablet_id, const std::string& version) { + std::vector scan_ranges; + TScanRangeParams scan_range; + TPaloScanRange palo_scan_range; + palo_scan_range.__set_tablet_id(tablet_id); + palo_scan_range.__set_version(version); + scan_range.scan_range.__set_palo_scan_range(palo_scan_range); + scan_ranges.push_back(scan_range); + return scan_ranges; +} + +TQueryCacheParam make_cache_param(int64_t tablet_id) { + TQueryCacheParam cache_param; + cache_param.__set_digest("runtime_test_digest"); + cache_param.tablet_to_range.insert({tablet_id, "range"}); + // FE always sets the entry limits; keep them roomy so decision tests do + // not trip the write-back feasibility check unintentionally. + cache_param.__set_entry_max_bytes(1024 * 1024); + cache_param.__set_entry_max_rows(100000); + return cache_param; +} + +void insert_entry(QueryCache* cache, const std::string& cache_key, int64_t version, + int64_t delta_count) { + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({1, 2, 3}); + cache->insert(cache_key, version, result, {0}, 1, delta_count); +} + +} // namespace + +TEST_F(QueryCacheTest, runtime_decision_miss_and_idempotent) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + QueryCacheRuntime runtime(make_cache_param(42), cache.get()); + + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->current_version, 100); + EXPECT_FALSE(decision->handle.valid()); + + // Idempotent: the second caller (e.g. the other operator) observes the + // same decision object. + auto decision2 = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision.get(), decision2.get()); +} + +TEST_F(QueryCacheTest, runtime_decision_invalid_key) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + // Tablet 42 is missing from tablet_to_range, so build_cache_key fails and + // the query degrades to an uncached scan instead of failing. + TQueryCacheParam cache_param; + cache_param.__set_digest("runtime_test_digest"); + cache_param.tablet_to_range.insert({43, "range"}); + QueryCacheRuntime runtime(cache_param, cache.get()); + + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_FALSE(decision->key_valid); + // Every caller shares one immutable invalid decision (and one log line). + EXPECT_EQ(decision.get(), runtime.get_or_make_decision(scan_ranges).get()); +} + +TEST_F(QueryCacheTest, runtime_decision_hit) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(cache.get(), cache_key, 100, 0); + + QueryCacheRuntime runtime(cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::HIT); + EXPECT_TRUE(decision->handle.valid()); + EXPECT_EQ(decision->handle.get_cache_version(), 100); +} + +TEST_F(QueryCacheTest, runtime_decision_force_refresh) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(cache.get(), cache_key, 100, 0); + + cache_param.__set_force_refresh_query_cache(true); + QueryCacheRuntime runtime(cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + // Recompute and write back even though a fresh entry exists. + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_FALSE(decision->handle.valid()); +} + +TEST_F(QueryCacheTest, runtime_decision_binlog_scan) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(cache.get(), cache_key, 100, 0); + + QueryCacheRuntime runtime(cache_param, cache.get()); + runtime.disable_for_binlog_scan(); + auto decision = runtime.get_or_make_decision(scan_ranges); + // A binlog scan must neither serve the cached entry nor write back. + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_FALSE(decision->key_valid); +} + +TEST_F(QueryCacheTest, runtime_decision_stale_without_incremental) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(cache.get(), cache_key, 50, 0); + + // allow_incremental unset: a stale entry is a plain miss (full recompute). + QueryCacheRuntime runtime(cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_FALSE(decision->handle.valid()); +} + +TEST_F(QueryCacheTest, runtime_decision_stale_incremental_fallbacks) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + + { + // The entry is newer than the version this replica is asked to read: + // never usable, fall back to a full scan of the requested version. + insert_entry(cache.get(), cache_key, 200, 0); + QueryCacheRuntime runtime(cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->incremental_fallback_reason, "cached entry is newer"); + } + { + // Too many accumulated incremental merges: force a full recompute to + // compact the entry. (Checked before any tablet access.) + insert_entry(cache.get(), cache_key, 50, config::query_cache_max_incremental_merge_count); + QueryCacheRuntime runtime(cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->incremental_fallback_reason, + "delta count reached compaction threshold"); + } + { + // Use a tablet id that exists nowhere, so fetching the tablet fails + // regardless of whether some other suite left a storage engine behind + // in this test process, and the decision safely falls back to MISS. + auto missing_scan_ranges = make_scan_ranges(424242424242, "100"); + auto missing_cache_param = make_cache_param(424242424242); + missing_cache_param.__set_allow_incremental(true); + std::string missing_cache_key; + int64_t missing_version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(missing_scan_ranges, missing_cache_param, + &missing_cache_key, &missing_version) + .ok()); + insert_entry(cache.get(), missing_cache_key, 50, 0); + QueryCacheRuntime runtime(missing_cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(missing_scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_FALSE(decision->handle.valid()); + EXPECT_EQ(decision->incremental_fallback_reason, "tablet not found"); + } +} + +TEST_F(QueryCacheTest, lookup_any_version_and_delta_count) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + + QueryCacheHandle handle; + EXPECT_FALSE(cache->lookup_any_version(cache_key, &handle)); + + int64_t write_backs_before = DorisMetrics::instance()->query_cache_write_back_total->value(); + insert_entry(cache.get(), cache_key, 50, 3); + EXPECT_EQ(DorisMetrics::instance()->query_cache_write_back_total->value(), + write_backs_before + 1); + QueryCacheHandle handle2; + EXPECT_TRUE(cache->lookup_any_version(cache_key, &handle2)); + EXPECT_EQ(handle2.get_cache_version(), 50); + EXPECT_EQ(handle2.get_cache_delta_count(), 3); + // insert_entry stores one 3-row block with cache_size 1. + EXPECT_EQ(handle2.get_cache_total_rows(), 3); + EXPECT_EQ(handle2.get_cache_total_bytes(), 1); + + // Exact-version lookup still rejects the stale entry. + QueryCacheHandle handle3; + EXPECT_FALSE(cache->lookup(cache_key, 100, &handle3)); +} + +TEST_F(QueryCacheTest, runtime_default_global_cache) { + // The single-argument constructor falls back to the global instance + // (whatever it is in this test environment). + QueryCacheRuntime runtime(make_cache_param(42)); + EXPECT_EQ(runtime.cache(), QueryCache::instance()); +} + +TEST_F(QueryCacheTest, take_delta_read_source) { + auto decision = std::make_shared(); + decision->_delta_read_sources[42] = std::make_unique(); + + EXPECT_EQ(decision->take_delta_read_source(41), nullptr); + auto source = decision->take_delta_read_source(42); + EXPECT_NE(source, nullptr); + // Each read source can be consumed exactly once. + EXPECT_EQ(decision->take_delta_read_source(42), nullptr); +} + +TEST_F(QueryCacheTest, runtime_decision_stale_incremental_cloud_mode) { + std::unique_ptr cache(QueryCache::create_global_cache(1024 * 1024)); + auto scan_ranges = make_scan_ranges(42, "100"); + auto cache_param = make_cache_param(42); + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(cache.get(), cache_key, 50, 0); + + // Incremental merge only supports local storage for now. + std::string saved_deploy_mode = config::deploy_mode; + config::deploy_mode = "cloud"; + QueryCacheRuntime runtime(cache_param, cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + config::deploy_mode = saved_deploy_mode; + + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->incremental_fallback_reason, "cloud mode"); +} + +// Exercises the per-tablet part of the incremental decision against a real +// (metadata-only) tablet registered in a real storage engine: capturing the +// delta read source never touches segment files, so no data is needed. +class QueryCacheIncrementalTest : public testing::Test { +protected: + static constexpr int64_t kTabletId = 15673; + static constexpr const char* kTestDir = "/ut_dir/query_cache_incremental_test"; + + void SetUp() override { + char buffer[1024]; + EXPECT_NE(getcwd(buffer, sizeof(buffer)), nullptr); + _absolute_dir = std::string(buffer) + kTestDir; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(_absolute_dir).ok()); + EXPECT_TRUE(io::global_local_filesystem()->create_directory(_absolute_dir).ok()); + + auto engine = std::make_unique(EngineOptions {}); + _engine = engine.get(); + ExecEnv::GetInstance()->set_storage_engine(std::move(engine)); + _data_dir = std::make_unique(*_engine, _absolute_dir); + static_cast(_data_dir->init()); + _cache.reset(QueryCache::create_global_cache(1024 * 1024)); + } + + void TearDown() override { + _cache.reset(); + _data_dir.reset(); + ExecEnv::GetInstance()->set_storage_engine(nullptr); + _engine = nullptr; + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(_absolute_dir).ok()); + } + + void init_rs_meta(RowsetMetaSharedPtr& rs_meta, const TabletMetaSharedPtr& tablet_meta, + int64_t start, int64_t end, bool with_delete_predicate) { + static const std::string json_rowset_meta = R"({ + "rowset_id": 540081, + "tablet_id": 15673, + "txn_id": 4042, + "tablet_schema_hash": 567997577, + "rowset_type": "BETA_ROWSET", + "rowset_state": "VISIBLE", + "start_version": 2, + "end_version": 2, + "num_rows": 3929, + "total_disk_size": 84699, + "data_disk_size": 84464, + "index_disk_size": 235, + "empty": false, + "load_id": { + "hi": -5350970832824939812, + "lo": -6717994719194512122 + }, + "creation_time": 1553765670 + })"; + RowsetMetaPB rowset_meta_pb; + json2pb::JsonToProtoMessage(json_rowset_meta, &rowset_meta_pb); + rowset_meta_pb.set_start_version(start); + rowset_meta_pb.set_end_version(end); + // One distinct rowset id per rowset (the json template repeats one id): + // the merge-on-write history-rewrite check tells baseline and delta + // rowsets apart by rowset id, see rowset_id_for(). + rowset_meta_pb.set_rowset_id(540081 + start); + rowset_meta_pb.set_creation_time(10000); + if (with_delete_predicate) { + DeletePredicatePB* delete_predicate = rowset_meta_pb.mutable_delete_predicate(); + delete_predicate->set_version(static_cast(start)); + delete_predicate->add_sub_predicates("k1='1'"); + } + rs_meta->init_from_pb(rowset_meta_pb); + rs_meta->set_tablet_schema(tablet_meta->tablet_schema()); + } + + TabletSharedPtr create_tablet(TKeysType::type keys_type, bool enable_merge_on_write, + const std::vector>& versions, + int64_t delete_predicate_start_version = -1) { + TTabletSchema schema; + schema.keys_type = keys_type; + TabletMetaSharedPtr tablet_meta(new TabletMeta( + 1, 2, kTabletId, 15674, 4, 5, schema, 6, {{7, 8}}, UniqueId(9, 10), + TTabletType::TABLET_TYPE_DISK, TCompressionType::LZ4F, 0, enable_merge_on_write)); + for (auto [start, end] : versions) { + RowsetMetaSharedPtr rs_meta(new RowsetMeta()); + init_rs_meta(rs_meta, tablet_meta, start, end, start == delete_predicate_start_version); + static_cast(tablet_meta->add_rs_meta(rs_meta)); + } + auto tablet = std::make_shared(*_engine, std::move(tablet_meta), _data_dir.get()); + static_cast(tablet->init()); + auto& tablet_map = _engine->tablet_manager()->_get_tablet_map(kTabletId); + tablet_map[kTabletId] = tablet; + return tablet; + } + + // The rowset id a fixture rowset starting at `start_version` ends up with, + // matching init_rs_meta() above (numeric ids use the v1 format). + static RowsetId rowset_id_for(int64_t start_version) { + RowsetId id; + id.init(540081 + start_version); + return id; + } + + // Common scenario: a stale entry of version 50 while the query reads + // version 100 with allow_incremental set. + std::shared_ptr make_stale_decision() { + auto scan_ranges = make_scan_ranges(kTabletId, "100"); + auto cache_param = make_cache_param(kTabletId); + cache_param.__set_allow_incremental(true); + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE( + QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(_cache.get(), cache_key, 50, 0); + QueryCacheRuntime runtime(cache_param, _cache.get()); + return runtime.get_or_make_decision(scan_ranges); + } + + std::string _absolute_dir; + StorageEngine* _engine = nullptr; + std::unique_ptr _data_dir; + std::unique_ptr _cache; +}; + +TEST_F(QueryCacheIncrementalTest, incremental_success) { + create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 80}, {81, 100}}); + int64_t stale_hits_before = DorisMetrics::instance()->query_cache_stale_hit_total->value(); + auto decision = make_stale_decision(); + EXPECT_EQ(DorisMetrics::instance()->query_cache_stale_hit_total->value(), + stale_hits_before + 1); + + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); + EXPECT_TRUE(decision->key_valid); + EXPECT_TRUE(decision->handle.valid()); + EXPECT_TRUE(decision->write_back_feasible); + EXPECT_TRUE(decision->incremental_fallback_reason.empty()); + EXPECT_EQ(decision->cached_version, 50); + EXPECT_EQ(decision->cached_delta_count, 0); + EXPECT_EQ(decision->current_version, 100); + + // The pre-captured read source covers exactly the delta (50, 100]. + auto source = decision->take_delta_read_source(kTabletId); + ASSERT_NE(source, nullptr); + EXPECT_EQ(source->rs_splits.size(), 2); + EXPECT_TRUE(source->delete_predicates.empty()); + // Consumable exactly once. + EXPECT_EQ(decision->take_delta_read_source(kTabletId), nullptr); +} + +TEST_F(QueryCacheIncrementalTest, incremental_write_back_infeasible) { + create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 100}}); + // The cached entry alone (3 rows) already exceeds entry_max_rows, so the + // merged entry could never be written back: still scan only the delta, but + // announce upfront that cloning blocks for a write back is pointless. + auto scan_ranges = make_scan_ranges(kTabletId, "100"); + auto cache_param = make_cache_param(kTabletId); + cache_param.__set_allow_incremental(true); + cache_param.__set_entry_max_rows(2); + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(_cache.get(), cache_key, 50, 0); + + QueryCacheRuntime runtime(cache_param, _cache.get()); + auto decision = runtime.get_or_make_decision(scan_ranges); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); + EXPECT_FALSE(decision->write_back_feasible); +} + +TEST_F(QueryCacheIncrementalTest, fallback_on_agg_keys) { + create_tablet(TKeysType::AGG_KEYS, false, {{0, 50}, {51, 100}}); + int64_t fallbacks_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); + auto decision = make_stale_decision(); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallbacks_before + 1); + // Storage-layer aggregation breaks "cached + delta == new snapshot". + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_FALSE(decision->handle.valid()); + EXPECT_EQ(decision->incremental_fallback_reason, "keys type not append-only"); +} + +TEST_F(QueryCacheIncrementalTest, fallback_on_merge_on_read) { + // Merge-on-read UNIQUE resolves duplicates by merging across rowsets at + // read time, so a delta-only scan cannot stand alone: always fall back. + create_tablet(TKeysType::UNIQUE_KEYS, false, {{0, 50}, {51, 100}}); + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_EQ(decision->incremental_fallback_reason, "keys type not append-only"); +} + +TEST_F(QueryCacheIncrementalTest, mow_pure_append_incremental) { + // Merge-on-write UNIQUE with no delete-bitmap entry in the delta window: + // the hourly-append pattern. Incremental merge is as safe as on DUP. + create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 80}, {81, 100}}); + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); + EXPECT_TRUE(decision->incremental_fallback_reason.empty()); + auto source = decision->take_delta_read_source(kTabletId); + ASSERT_NE(source, nullptr); + EXPECT_EQ(source->rs_splits.size(), 2); + // capture_read_source hands the delete bitmap to the delta scan, so rows + // replaced within the delta window itself are filtered by the reader. + EXPECT_NE(source->delete_bitmap, nullptr); +} + +TEST_F(QueryCacheIncrementalTest, mow_history_rewrite_falls_back) { + // A load inside the delta window marked a row of a baseline rowset as + // deleted (an upsert / backfill hit a pre-existing key): rows already + // folded into the cached entry cannot be subtracted, so fall back. + auto tablet = create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}); + tablet->tablet_meta()->delete_bitmap().add({rowset_id_for(0), 0, 60}, 7); + int64_t fallbacks_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); + auto decision = make_stale_decision(); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallbacks_before + 1); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_FALSE(decision->handle.valid()); + EXPECT_EQ(decision->incremental_fallback_reason, "delta rewrites history rows"); +} + +TEST_F(QueryCacheIncrementalTest, mow_irrelevant_bitmap_entries_ignored) { + // Delete-bitmap entries that cannot affect the cached snapshot must not + // spoil the incremental path: entries targeting the delta rowsets (a key + // written twice within the window, a lost sequence-column race or a delete + // sign on a window-local key), entries outside the version window (older + // dedup history, a concurrent load newer than the read version, pending + // entries at TEMP_VERSION_COMMON = 0) and empty bitmaps. + auto tablet = create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}); + auto& delete_bitmap = tablet->tablet_meta()->delete_bitmap(); + delete_bitmap.add({rowset_id_for(51), 0, 90}, 3); // targets the delta itself + delete_bitmap.add({rowset_id_for(0), 0, 40}, 1); // version <= cached + delete_bitmap.add({rowset_id_for(0), 0, 150}, 2); // version > current + delete_bitmap.add({rowset_id_for(0), 0, 0}, 4); // pending (TEMP_VERSION_COMMON) + delete_bitmap.set({rowset_id_for(0), 0, 60}, roaring::Roaring()); // empty bitmap + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); + EXPECT_TRUE(decision->incremental_fallback_reason.empty()); +} + +TEST_F(QueryCacheIncrementalTest, fallback_on_version_gap) { + // The whole history lives in one compacted rowset [0, 100]: no version + // path can serve (50, 100] alone, so capture fails and we fall back. + create_tablet(TKeysType::DUP_KEYS, false, {{0, 100}}); + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->incremental_fallback_reason, "delta versions not capturable"); +} + +TEST_F(QueryCacheIncrementalTest, fallback_on_capture_error) { + // Non-pathfinding capture failures surface as real errors even under the + // quiet option (a pathfinding miss instead returns an empty read source, + // see fallback_on_version_gap). The storage debug point simulates such a + // failure, e.g. a rowset object missing for a found path. + create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 100}}); + config::enable_debug_points = true; + DebugPoints::instance()->add_with_params("Tablet::capture_consistent_versions.inject_failure", + {{"tablet_id", "-2"}}); + auto decision = make_stale_decision(); + DebugPoints::instance()->remove("Tablet::capture_consistent_versions.inject_failure"); + config::enable_debug_points = false; + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->incremental_fallback_reason, "delta versions not capturable"); +} + +TEST_F(QueryCacheIncrementalTest, fallback_on_delete_predicate) { + // A delete predicate inside the delta logically removes rows that are + // already folded into the cached blocks; that cannot be merged away. + create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 60}, {61, 100}}, + /*delete_predicate_start_version=*/61); + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_EQ(decision->incremental_fallback_reason, "delta contains delete predicates"); +} + } // namespace doris diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/AggregationNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/AggregationNode.java index e5820c4e426755..786e026660af39 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/AggregationNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/AggregationNode.java @@ -89,6 +89,10 @@ public void unsetNeedsFinalize() { updateplanNodeName(); } + public boolean isNeedsFinalize() { + return needsFinalize; + } + // Used by new optimizer public void setUseStreamingPreagg(boolean useStreamingPreagg) { this.useStreamingPreagg = useStreamingPreagg; diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/normalize/QueryCacheNormalizer.java b/fe/fe-core/src/main/java/org/apache/doris/planner/normalize/QueryCacheNormalizer.java index b74ebd0e7c3aca..5212c5a5fd6533 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/normalize/QueryCacheNormalizer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/normalize/QueryCacheNormalizer.java @@ -21,6 +21,7 @@ package org.apache.doris.planner.normalize; import org.apache.doris.analysis.DescriptorTable; +import org.apache.doris.catalog.KeysType; import org.apache.doris.catalog.OlapTable; import org.apache.doris.catalog.Tablet; import org.apache.doris.common.Pair; @@ -98,6 +99,7 @@ private Optional setQueryCacheParam( queryCacheParam.setForceRefreshQueryCache(sessionVariable.isQueryCacheForceRefresh()); queryCacheParam.setEntryMaxBytes(sessionVariable.getQueryCacheEntryMaxBytes()); queryCacheParam.setEntryMaxRows(sessionVariable.getQueryCacheEntryMaxRows()); + queryCacheParam.setAllowIncremental(computeAllowIncremental(cachePoint, sessionVariable)); queryCacheParam.setOutputSlotMapping( cachePoint.cacheRoot.getOutputTupleIds() @@ -145,6 +147,74 @@ private Optional doComputeCachePoint(PlanNode planRoot) { return Optional.empty(); } + /** + * Decide whether BE may serve a stale cache entry by scanning only the delta + * rowsets since the cached version and emitting them together with the cached + * partial aggregation blocks (the upstream aggregation merges both). + * + *

This is only correct when all of the following hold: + *

    + *
  • The cache point aggregation does not finalize: its output is a partial + * state that an upstream aggregation always merges, so emitting the cached + * blocks and the delta blocks side by side yields the correct result. A + * finalized output has no downstream merge, and the two emissions would + * produce duplicated group keys.
    • + *
  • The cache point aggregates the raw detail rows directly (its child is + * the olap scan node): with another aggregation in between, that inner + * aggregation would see only the delta rows during an incremental run, and + * its finalized output over the delta is not a mergeable complement of its + * output over the cached snapshot.
    • + *
  • The scanned index is append-only, guaranteeing "cached snapshot + + * delta rowsets == new snapshot": either DUP_KEYS, or merge-on-write + * UNIQUE_KEYS, for which BE additionally verifies per tablet (through the + * delete bitmap of the delta window) that no pre-existing key was rewritten + * and falls back otherwise. Merge-on-read UNIQUE resolves duplicates by + * merging across rowsets at read time, so a delta-only scan cannot stand + * alone there; AGG tables merge rows inside the storage layer likewise. + * DELETE predicates are handled on BE: a delta containing delete predicates + * falls back to a full recompute.
    • + *
+ */ + private boolean computeAllowIncremental(CachePoint cachePoint, SessionVariable sessionVariable) { + if (!sessionVariable.getEnableQueryCacheIncremental()) { + return false; + } + // The cache point is always an aggregation node (see doComputeCachePoint). + if (((AggregationNode) cachePoint.cacheRoot).isNeedsFinalize()) { + return false; + } + // The cached partial state and the delta partial state merge correctly + // only when the cache point aggregates the raw detail rows directly. + // With a nested cache point (partial agg over a finalized/deduplicating + // agg over scan), the inner agg sees only the delta rows during an + // incremental run, so its finalized output over the delta is NOT a + // mergeable complement of its output over the cached snapshot (e.g. + // "group by cnt" buckets computed from partial counts are simply wrong). + if (!(cachePoint.cacheRoot.getChild(0) instanceof OlapScanNode)) { + return false; + } + OlapScanNode scanNode = (OlapScanNode) cachePoint.cacheRoot.getChild(0); + OlapTable olapTable = scanNode.getOlapTable(); + long selectIndexId = scanNode.getSelectedIndexId() == -1 + ? olapTable.getBaseIndexId() + : scanNode.getSelectedIndexId(); + // Note: judged on the selected index, not the base table: a DUP table + // may serve the query from an aggregated materialized view, whose data + // is no longer append-only. + KeysType keysType = olapTable.getKeysTypeByIndexId(selectIndexId); + if (keysType == KeysType.DUP_KEYS) { + return true; + } + // A merge-on-write UNIQUE index is append-only as long as a load does + // not touch pre-existing keys, which covers the common "hourly append + // plus occasional backfill" pattern: BE verifies per tablet through + // the delete bitmap of the delta window and falls back to a full + // recompute for the rare load that rewrites history. A merge-on-read + // UNIQUE index resolves duplicates by merging across rowsets at read + // time, so a delta-only scan cannot stand alone there. + return keysType == KeysType.UNIQUE_KEYS && olapTable.getEnableUniqueKeyMergeOnWrite(); + } + private List normalizePlanTree(ConnectContext context, CachePoint cachePoint) { List normalizedPlans = new ArrayList<>(); doNormalizePlanTree(context, cachePoint.digestRoot, normalizedPlans); diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index edae724ae165f0..51d4b758d4ed20 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -202,6 +202,7 @@ public class SessionVariable implements Serializable, Writable { public static final String ENABLE_SQL_CACHE = "enable_sql_cache"; public static final String ENABLE_HIVE_SQL_CACHE = "enable_hive_sql_cache"; public static final String ENABLE_QUERY_CACHE = "enable_query_cache"; + public static final String ENABLE_QUERY_CACHE_INCREMENTAL = "enable_query_cache_incremental"; public static final String QUERY_CACHE_FORCE_REFRESH = "query_cache_force_refresh"; public static final String QUERY_CACHE_ENTRY_MAX_BYTES = "query_cache_entry_max_bytes"; public static final String QUERY_CACHE_ENTRY_MAX_ROWS = "query_cache_entry_max_rows"; @@ -1544,6 +1545,36 @@ public enum IgnoreSplitType { @VarAttrDef.VarAttr(name = ENABLE_QUERY_CACHE, fuzzy = true) public boolean enableQueryCache = false; + // Allow BE to reuse a stale query cache entry by scanning only the delta + // rowsets since the cached version and merging them with the cached partial + // aggregation blocks. Only takes effect when the cache point aggregation is + // non-finalize (its output is merged again upstream) and the selected index + // is append-only: DUP_KEYS, or merge-on-write UNIQUE_KEYS whose delta did + // not rewrite pre-existing keys (BE checks the delete bitmap per tablet); + // BE falls back to a full recompute whenever the delta cannot be captured + // (compacted away), contains delete predicates or rewrites history rows. + // Experimental: shown as `experimental_enable_query_cache_incremental` and + // settable with or without the prefix; to be promoted to EXPERIMENTAL_ONLINE + // once it graduates. + @VarAttrDef.VarAttr(name = ENABLE_QUERY_CACHE_INCREMENTAL, + varType = VariableAnnotation.EXPERIMENTAL, needForward = true, + description = {"是否允许 BE 以增量合并的方式复用过期的 Query Cache 条目：只扫描缓存版本之后的" + + "增量 rowset，并与缓存的聚合中间结果一起交给上游合并。仅对聚合直压扫描且不做 finalize " + + "的缓存点生效，且选中索引须为追加写：DUP_KEYS 表，或增量窗口内未改写既有主键的写时合并" + + "（merge-on-write）UNIQUE_KEYS 表（BE 按 tablet 检查 delete bitmap）；增量不可捕获（如" + + "已被 compaction 合并）、含 DELETE 谓词或改写了历史行时自动回退全量重算。需与 " + + "enable_query_cache 同时开启。", + "Whether BE may reuse a stale query cache entry by incremental merge: scan only" + + " the delta rowsets since the cached version and emit them together with the" + + " cached partial aggregation blocks for the upstream merge. Only takes effect" + + " when the cache point is a non-finalize aggregation directly over the scan" + + " and the selected index is append-only: DUP_KEYS, or merge-on-write" + + " UNIQUE_KEYS whose delta did not rewrite pre-existing keys (BE checks the" + + " delete bitmap per tablet); falls back to a full recompute whenever the" + + " delta cannot be captured (e.g. compacted away), contains delete predicates" + + " or rewrites history rows. Requires enable_query_cache."}) + public boolean enableQueryCacheIncremental = false; + @VarAttrDef.VarAttr(name = QUERY_CACHE_FORCE_REFRESH) private boolean queryCacheForceRefresh = false; @@ -4657,6 +4688,14 @@ public void setEnableQueryCache(boolean enableQueryCache) { this.enableQueryCache = enableQueryCache; } + public boolean getEnableQueryCacheIncremental() { + return enableQueryCacheIncremental; + } + + public void setEnableQueryCacheIncremental(boolean enableQueryCacheIncremental) { + this.enableQueryCacheIncremental = enableQueryCacheIncremental; + } + public boolean isQueryCacheForceRefresh() { return queryCacheForceRefresh; } diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryCacheNormalizerTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryCacheNormalizerTest.java index e8bf8b628c3171..d7cb0e7d94c434 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryCacheNormalizerTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryCacheNormalizerTest.java @@ -117,7 +117,29 @@ protected void runBeforeAll() throws Exception { + "distributed by hash(k1) buckets 3\n" + "properties('replication_num' = '1')"; - createTables(nonPart, part1, part2, multiLeveParts, variantTable); + String uniqueMowTable = "create table db1.uniq_mow(" + + " k1 int,\n" + + " v1 int)\n" + + "UNIQUE KEY(k1)\n" + + "distributed by hash(k1) buckets 3\n" + + "properties('replication_num' = '1', 'enable_unique_key_merge_on_write' = 'true')"; + + String uniqueMorTable = "create table db1.uniq_mor(" + + " k1 int,\n" + + " v1 int)\n" + + "UNIQUE KEY(k1)\n" + + "distributed by hash(k1) buckets 3\n" + + "properties('replication_num' = '1', 'enable_unique_key_merge_on_write' = 'false')"; + + String aggTable = "create table db1.agg_tbl(" + + " k1 int,\n" + + " v1 int sum)\n" + + "AGGREGATE KEY(k1)\n" + + "distributed by hash(k1) buckets 3\n" + + "properties('replication_num' = '1')"; + + createTables(nonPart, part1, part2, multiLeveParts, variantTable, uniqueMowTable, + uniqueMorTable, aggTable); connectContext.getSessionVariable().setDisableNereidsRules("PRUNE_EMPTY_PARTITION"); connectContext.getSessionVariable().setEnableQueryCache(true); @@ -382,6 +404,72 @@ public void testVariantSubColumnDigest() throws Exception { Assertions.assertEquals(digest1, digest3); } + @Test + public void testAllowIncremental() throws Exception { + // Switch off (the default): never allow incremental merge. + Assertions.assertFalse(connectContext.getSessionVariable().getEnableQueryCacheIncremental()); + TQueryCacheParam withoutSwitch = getQueryCacheParam( + "select k2, sum(v1) as v from db1.part1 group by k2"); + Assertions.assertFalse(withoutSwitch.allow_incremental); + + connectContext.getSessionVariable().setEnableQueryCacheIncremental(true); + try { + // DUP_KEYS with a two-phase aggregation (group by a non-distribution + // column): the cache point is the non-finalize first phase, whose + // output is merged again upstream, so incremental merge is safe. + TQueryCacheParam dupTwoPhase = getQueryCacheParam( + "select k2, sum(v1) as v from db1.part1 group by k2"); + Assertions.assertTrue(dupTwoPhase.allow_incremental); + + // One-phase aggregation finalizes inside the cached fragment: its + // output has no downstream merge, so emitting cached and delta + // blocks side by side would duplicate group keys. + TQueryCacheParam onePhase = phaseAgg(1, () -> getQueryCacheParam( + "select k1, sum(v1) as v from db1.non_part group by k1")); + Assertions.assertFalse(onePhase.allow_incremental); + + // UNIQUE merge-on-write is append-only as long as loads do not + // touch pre-existing keys; FE grants the capability and BE checks + // the delete bitmap of the delta window per tablet. Group by a + // non-distribution column so the aggregation stays two-phase and + // the decision reaches the keys-type check. + TQueryCacheParam uniqueMow = getQueryCacheParam( + "select v1, count(*) as v from db1.uniq_mow group by v1"); + Assertions.assertTrue(uniqueMow.allow_incremental); + + // UNIQUE merge-on-read resolves duplicates by merging across + // rowsets at read time: a delta-only scan cannot stand alone. + TQueryCacheParam uniqueMor = getQueryCacheParam( + "select v1, count(*) as v from db1.uniq_mor group by v1"); + Assertions.assertFalse(uniqueMor.allow_incremental); + + // AGG_KEYS merges rows inside the storage layer. + TQueryCacheParam aggKeys = getQueryCacheParam( + "select v1, count(*) as v from db1.agg_tbl group by v1"); + Assertions.assertFalse(aggKeys.allow_incremental); + + // Agg over Agg inside one fragment (distinct aggregation grouped by + // the distribution column): the cache point is not directly on the + // scan (and finalizes here as well), so incremental is not allowed. + TQueryCacheParam distinctAgg = getQueryCacheParam( + "select k1, count(distinct k2) from db1.non_part group by k1"); + Assertions.assertFalse(distinctAgg.allow_incremental); + + // Nested cache point: a non-finalize partial agg over a finalized + // colocate agg over scan. The inner agg would see only the delta + // rows during an incremental run, so its finalized output is not a + // mergeable complement of the cached snapshot -- must be rejected + // even though the cache point itself does not finalize. + TQueryCacheParam nestedAgg = getQueryCacheParam( + "select cnt, count(*) from" + + " (select k1, count(*) cnt from db1.non_part group by k1) x" + + " group by cnt"); + Assertions.assertFalse(nestedAgg.allow_incremental); + } finally { + connectContext.getSessionVariable().setEnableQueryCacheIncremental(false); + } + } + private String getDigest(String sql) throws Exception { return Hex.encodeHexString(getQueryCacheParam(sql).digest); } diff --git a/gensrc/thrift/QueryCache.thrift b/gensrc/thrift/QueryCache.thrift index 048b27f4572dd1..67c31cd4bc6120 100644 --- a/gensrc/thrift/QueryCache.thrift +++ b/gensrc/thrift/QueryCache.thrift @@ -50,4 +50,31 @@ struct TQueryCacheParam { 6: optional i64 entry_max_bytes 7: optional i64 entry_max_rows -} \ No newline at end of file + + // Whether BE is allowed to serve a stale cache entry by incremental merge: + // when the cached version is behind the current version, BE may scan only the + // delta rowsets in (cached_version, current_version], produce the partial + // aggregation of the delta, emit it together with the cached partial blocks + // (the upstream merge aggregation combines both), and write the merged entry + // back with the new version. + // + // FE only sets this to true when all of the following hold, otherwise the + // "cached + delta" union would not equal the new snapshot or could not be + // merged safely: + // - the scanned index is append-only: DUP_KEYS, or merge-on-write + // UNIQUE_KEYS (BE verifies per tablet, through the delete bitmap of the + // delta window, that no pre-existing key was rewritten); merge-on-read + // UNIQUE resolves duplicates while reading and AGG tables merge rows in + // the storage layer, so those always fall back + // - the cache point aggregation does not finalize (its output is a partial + // state that is always merged again by an upstream aggregation, so the + // cached blocks and the delta blocks can be emitted side by side) + // - the cache point aggregates the raw detail rows directly (its child is + // the olap scan node); with a nested aggregation the inner finalized agg + // would see only the delta rows, whose output is not a mergeable + // complement of the cached snapshot + // BE additionally falls back to a full recompute when the delta version path + // cannot be captured (e.g. merged away by compaction), when the delta + // contains delete predicates, or when the delta rewrites history rows. + 8: optional bool allow_incremental +} diff --git a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy new file mode 100644 index 00000000000000..9a0b32de639fb1 --- /dev/null +++ b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy @@ -0,0 +1,183 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Correctness of the query cache incremental merge: when a partition keeps +// receiving hourly loads, a stale cache entry is reused by scanning only the +// delta rowsets since the cached version and merging them with the cached +// partial aggregation blocks. Every query below is checked against the same +// query with the cache disabled, so the suite passes in any environment +// (including those where incremental merge falls back to a full recompute, +// e.g. cloud mode) while exercising the incremental path on local storage. +suite("query_cache_incremental") { + def tableName = "test_query_cache_incremental" + def uniqueTableName = "test_query_cache_incremental_mow" + def querySql = """ + SELECT + url, + SUM(cost) AS total_cost, + COUNT(*) AS cnt + FROM ${tableName} + WHERE dt >= '2026-01-01' + AND dt < '2026-01-15' + GROUP BY url + """ + + def normalize = { rows -> + return rows.collect { row -> row.collect { col -> String.valueOf(col) }.join("|") }.sort() + } + + // Compare the cached query result against the uncached one, twice: the + // first cached run may fill or incrementally merge the entry, the second + // one should serve it. + def checkConsistency = { String sqlText -> + sql "set enable_query_cache=false" + def expected = normalize(sql(sqlText)) + sql "set enable_query_cache=true" + assertEquals(expected, normalize(sql(sqlText))) + assertEquals(expected, normalize(sql(sqlText))) + } + + sql "set enable_nereids_distribute_planner=true" + sql "set enable_query_cache=true" + sql "set enable_query_cache_incremental=true" + sql "set parallel_pipeline_task_num=3" + sql "set enable_sql_cache=false" + + sql "DROP TABLE IF EXISTS ${tableName}" + sql """ + CREATE TABLE ${tableName} ( + dt DATE, + user_id INT, + url STRING, + cost BIGINT + ) + ENGINE=OLAP + DUPLICATE KEY(dt, user_id) + PARTITION BY RANGE(dt) + ( + PARTITION p20260101 VALUES LESS THAN ("2026-01-05"), + PARTITION p20260105 VALUES LESS THAN ("2026-01-10"), + PARTITION p20260110 VALUES LESS THAN ("2026-01-15") + ) + DISTRIBUTED BY HASH(user_id) BUCKETS 3 + PROPERTIES + ( + "replication_num" = "1" + ) + """ + + sql """ + INSERT INTO ${tableName} VALUES + ('2026-01-01',1,'/a',10), + ('2026-01-01',2,'/b',20), + ('2026-01-02',3,'/c',30), + ('2026-01-06',1,'/a',15), + ('2026-01-07',3,'/c',35), + ('2026-01-11',1,'/a',50), + ('2026-01-12',3,'/c',70) + """ + + // The query cache participates in this plan. + explain { + sql(querySql) + contains("DIGEST") + } + + // Fill the cache, then serve from it. + checkConsistency(querySql) + + // Simulate the hourly load pattern: only the latest partition receives new + // data, so the entries of the older partitions stay valid while the hot + // partition entry is stale and gets incrementally merged. Crossing + // query_cache_max_incremental_merge_count (BE config, default 8) also + // exercises the forced full recompute that compacts the entry. + for (int i = 1; i <= 10; i++) { + sql """ + INSERT INTO ${tableName} VALUES + ('2026-01-13',${i},'/a',${i}), + ('2026-01-13',${100 + i},'/inc',${10 * i}) + """ + checkConsistency(querySql) + } + + // A delete in the delta cannot be merged into the cached partial result; + // the query must fall back to a full recompute and stay correct. + sql "DELETE FROM ${tableName} PARTITION p20260110 WHERE user_id = 1" + checkConsistency(querySql) + + sql """ + INSERT INTO ${tableName} VALUES + ('2026-01-14',999,'/after-delete',1) + """ + checkConsistency(querySql) + + sql "DROP TABLE IF EXISTS ${tableName}" + + // A UNIQUE merge-on-write table takes the incremental path as long as the + // loads only append new keys (the delete bitmap of the delta window stays + // empty); a load that rewrites a pre-existing key falls back to one full + // recompute and re-bases the entry, after which pure appends are + // incremental again. Results must stay correct in every phase. + sql "DROP TABLE IF EXISTS ${uniqueTableName}" + sql """ + CREATE TABLE ${uniqueTableName} ( + dt DATE, + user_id INT, + cost BIGINT + ) + ENGINE=OLAP + UNIQUE KEY(dt, user_id) + PARTITION BY RANGE(dt) + ( + PARTITION p20260101 VALUES LESS THAN ("2026-01-05"), + PARTITION p20260105 VALUES LESS THAN ("2026-01-10") + ) + DISTRIBUTED BY HASH(user_id) BUCKETS 3 + PROPERTIES + ( + "replication_num" = "1", + "enable_unique_key_merge_on_write" = "true" + ) + """ + def uniqueQuerySql = """ + SELECT dt, SUM(cost) AS total_cost + FROM ${uniqueTableName} + GROUP BY dt + """ + sql """ + INSERT INTO ${uniqueTableName} VALUES + ('2026-01-01',1,10), + ('2026-01-02',2,20), + ('2026-01-06',3,30) + """ + checkConsistency(uniqueQuerySql) + // The hourly-append pattern on a primary-key table: every load only adds + // brand-new keys, so the stale entry merges incrementally. + for (int i = 1; i <= 3; i++) { + sql "INSERT INTO ${uniqueTableName} VALUES ('2026-01-06',${100 + i},${i})" + checkConsistency(uniqueQuerySql) + } + // A backfill rewrites history through the delete bitmap: user_id 1 gets a + // new cost, which forces one full recompute that re-bases the entry. + sql "INSERT INTO ${uniqueTableName} VALUES ('2026-01-01',1,100)" + checkConsistency(uniqueQuerySql) + // After the re-base, pure appends take the incremental path again. + sql "INSERT INTO ${uniqueTableName} VALUES ('2026-01-06',200,7)" + checkConsistency(uniqueQuerySql) + + sql "DROP TABLE IF EXISTS ${uniqueTableName}" +} From 26484d76c5a1d0deb5d44ef2ee93d6f8aae531d0 Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Mon, 13 Jul 2026 21:09:01 +0800 Subject: [PATCH 2/9] [test](query-cache) Address review comments on the incremental merge tests Prove the incremental path in the regression through BE metrics: the first append round on the duplicate-key and the merge-on-write table must increase query_cache_stale_hit_total (the hot partition holds two data rowsets at that point, so neither the merge-count threshold nor a background compaction can interfere), and the delete-predicate and MoW backfill phases must each increase query_cache_incremental_fallback_total. Both counters only move when enable_query_cache_incremental is on (default off, this suite is the only one that sets it), and the assertions are one-sided deltas summed across all backends, so concurrent suites cannot break them. Cloud mode keeps the plain consistency checks since it always falls back by design. Align the suite with the regression standards: hardcoded table names, order_qt snapshots at each stable phase backed by a generated .out, and tables left in place after the run for debugging. Stop discarding setup statuses in the BE UT fixture: fatal assertions in SetUp() that print the failed Status, non-fatal checks with an early return in create_tablet() before the tablet is registered into the tablet map, null checks at the dereferencing call sites, and assertions on the two previously discarded booleans in init_rs_meta(). --- be/test/exec/pipeline/query_cache_test.cpp | 35 +++-- .../cache/query_cache_incremental.out | 29 ++++ .../cache/query_cache_incremental.groovy | 133 ++++++++++++++---- 3 files changed, 158 insertions(+), 39 deletions(-) create mode 100644 regression-test/data/query_p0/cache/query_cache_incremental.out diff --git a/be/test/exec/pipeline/query_cache_test.cpp b/be/test/exec/pipeline/query_cache_test.cpp index c6c0e08e81aad7..89895ab06f2db1 100644 --- a/be/test/exec/pipeline/query_cache_test.cpp +++ b/be/test/exec/pipeline/query_cache_test.cpp @@ -578,18 +578,24 @@ class QueryCacheIncrementalTest : public testing::Test { static constexpr int64_t kTabletId = 15673; static constexpr const char* kTestDir = "/ut_dir/query_cache_incremental_test"; + // Fatal assertions: every test in this fixture dereferences the storage + // engine and the data dir, so a failed setup must not fall through to the + // test body (TearDown still runs and tolerates the partial state). void SetUp() override { char buffer[1024]; - EXPECT_NE(getcwd(buffer, sizeof(buffer)), nullptr); + ASSERT_NE(getcwd(buffer, sizeof(buffer)), nullptr); _absolute_dir = std::string(buffer) + kTestDir; - EXPECT_TRUE(io::global_local_filesystem()->delete_directory(_absolute_dir).ok()); - EXPECT_TRUE(io::global_local_filesystem()->create_directory(_absolute_dir).ok()); + Status st = io::global_local_filesystem()->delete_directory(_absolute_dir); + ASSERT_TRUE(st.ok()) << st; + st = io::global_local_filesystem()->create_directory(_absolute_dir); + ASSERT_TRUE(st.ok()) << st; auto engine = std::make_unique(EngineOptions {}); _engine = engine.get(); ExecEnv::GetInstance()->set_storage_engine(std::move(engine)); _data_dir = std::make_unique(*_engine, _absolute_dir); - static_cast(_data_dir->init()); + st = _data_dir->init(); + ASSERT_TRUE(st.ok()) << st; _cache.reset(QueryCache::create_global_cache(1024 * 1024)); } @@ -598,7 +604,8 @@ class QueryCacheIncrementalTest : public testing::Test { _data_dir.reset(); ExecEnv::GetInstance()->set_storage_engine(nullptr); _engine = nullptr; - EXPECT_TRUE(io::global_local_filesystem()->delete_directory(_absolute_dir).ok()); + Status st = io::global_local_filesystem()->delete_directory(_absolute_dir); + EXPECT_TRUE(st.ok()) << st; } void init_rs_meta(RowsetMetaSharedPtr& rs_meta, const TabletMetaSharedPtr& tablet_meta, @@ -624,7 +631,7 @@ class QueryCacheIncrementalTest : public testing::Test { "creation_time": 1553765670 })"; RowsetMetaPB rowset_meta_pb; - json2pb::JsonToProtoMessage(json_rowset_meta, &rowset_meta_pb); + ASSERT_TRUE(json2pb::JsonToProtoMessage(json_rowset_meta, &rowset_meta_pb)); rowset_meta_pb.set_start_version(start); rowset_meta_pb.set_end_version(end); // One distinct rowset id per rowset (the json template repeats one id): @@ -637,7 +644,7 @@ class QueryCacheIncrementalTest : public testing::Test { delete_predicate->set_version(static_cast(start)); delete_predicate->add_sub_predicates("k1='1'"); } - rs_meta->init_from_pb(rowset_meta_pb); + ASSERT_TRUE(rs_meta->init_from_pb(rowset_meta_pb)); rs_meta->set_tablet_schema(tablet_meta->tablet_schema()); } @@ -652,10 +659,18 @@ class QueryCacheIncrementalTest : public testing::Test { for (auto [start, end] : versions) { RowsetMetaSharedPtr rs_meta(new RowsetMeta()); init_rs_meta(rs_meta, tablet_meta, start, end, start == delete_predicate_start_version); - static_cast(tablet_meta->add_rs_meta(rs_meta)); + Status st = tablet_meta->add_rs_meta(rs_meta); + EXPECT_TRUE(st.ok()) << st; + if (!st.ok()) { + return nullptr; + } } auto tablet = std::make_shared(*_engine, std::move(tablet_meta), _data_dir.get()); - static_cast(tablet->init()); + Status st = tablet->init(); + EXPECT_TRUE(st.ok()) << st; + if (!st.ok()) { + return nullptr; + } auto& tablet_map = _engine->tablet_manager()->_get_tablet_map(kTabletId); tablet_map[kTabletId] = tablet; return tablet; @@ -778,6 +793,7 @@ TEST_F(QueryCacheIncrementalTest, mow_history_rewrite_falls_back) { // deleted (an upsert / backfill hit a pre-existing key): rows already // folded into the cached entry cannot be subtracted, so fall back. auto tablet = create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}); + ASSERT_NE(tablet, nullptr); tablet->tablet_meta()->delete_bitmap().add({rowset_id_for(0), 0, 60}, 7); int64_t fallbacks_before = DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); @@ -797,6 +813,7 @@ TEST_F(QueryCacheIncrementalTest, mow_irrelevant_bitmap_entries_ignored) { // dedup history, a concurrent load newer than the read version, pending // entries at TEMP_VERSION_COMMON = 0) and empty bitmaps. auto tablet = create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}); + ASSERT_NE(tablet, nullptr); auto& delete_bitmap = tablet->tablet_meta()->delete_bitmap(); delete_bitmap.add({rowset_id_for(51), 0, 90}, 3); // targets the delta itself delete_bitmap.add({rowset_id_for(0), 0, 40}, 1); // version <= cached diff --git a/regression-test/data/query_p0/cache/query_cache_incremental.out b/regression-test/data/query_p0/cache/query_cache_incremental.out new file mode 100644 index 00000000000000..f1314984593fb9 --- /dev/null +++ b/regression-test/data/query_p0/cache/query_cache_incremental.out @@ -0,0 +1,29 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !dup_initial -- +/a 75 3 +/b 20 1 +/c 135 3 + +-- !dup_after_appends -- +/a 130 13 +/b 20 1 +/c 135 3 +/inc 550 10 + +-- !dup_final -- +/a 79 11 +/after-delete 1 1 +/b 20 1 +/c 135 3 +/inc 550 10 + +-- !mow_initial -- +2026-01-01 10 +2026-01-02 20 +2026-01-06 30 + +-- !mow_final -- +2026-01-01 100 +2026-01-02 20 +2026-01-06 43 + diff --git a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy index 9a0b32de639fb1..1e093a28e3128d 100644 --- a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy +++ b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy @@ -19,18 +19,20 @@ // receiving hourly loads, a stale cache entry is reused by scanning only the // delta rowsets since the cached version and merging them with the cached // partial aggregation blocks. Every query below is checked against the same -// query with the cache disabled, so the suite passes in any environment -// (including those where incremental merge falls back to a full recompute, -// e.g. cloud mode) while exercising the incremental path on local storage. +// query with the cache disabled, so results stay verified even where +// incremental merge falls back to a full recompute (e.g. cloud mode). On +// local storage the suite additionally proves through the BE metrics that +// the incremental path really fires: the early stale queries must increase +// query_cache_stale_hit_total, and the two designed fallback phases (a +// delete predicate in the delta, a merge-on-write backfill that rewrites +// history) must increase query_cache_incremental_fallback_total. suite("query_cache_incremental") { - def tableName = "test_query_cache_incremental" - def uniqueTableName = "test_query_cache_incremental_mow" def querySql = """ SELECT url, SUM(cost) AS total_cost, COUNT(*) AS cnt - FROM ${tableName} + FROM test_query_cache_incremental WHERE dt >= '2026-01-01' AND dt < '2026-01-15' GROUP BY url @@ -51,15 +53,66 @@ suite("query_cache_incremental") { assertEquals(expected, normalize(sql(sqlText))) } + // Sum a BE counter across all backends via the webserver /metrics text. + def sumBeMetric = { String metricName -> + def backendIdToIp = [:] + def backendIdToHttpPort = [:] + getBackendIpHttpPort(backendIdToIp, backendIdToHttpPort) + long total = 0 + backendIdToIp.each { backendId, ip -> + def text = new URL("http://${ip}:${backendIdToHttpPort[backendId]}/metrics").getText() + for (def line : text.split("\n")) { + if (line.startsWith("doris_be_${metricName} ")) { + total += Long.parseLong(line.substring(line.lastIndexOf(' ') + 1).trim()) + } + } + } + return total + } + + // Besides result consistency, prove on local storage that the stale entry + // was really merged incrementally (Mode::INCREMENTAL), not recomputed: + // only the incremental path increases query_cache_stale_hit_total, and no + // concurrent suite touches it because the session switch defaults to off. + // Cloud mode always falls back by design, so it only checks consistency. + // Residual caveat (here and in checkIncrementalFallback): a + // memory-pressure prune evicting the entry inside the tiny fill-to-assert + // window surfaces as a plain MISS and would fail the assertion; accepted + // as rare, and a rerun re-establishes the counters from fresh deltas. + def checkStaleIncremental = { String sqlText -> + if (isCloudMode()) { + checkConsistency(sqlText) + return + } + def before = sumBeMetric("query_cache_stale_hit_total") + checkConsistency(sqlText) + def after = sumBeMetric("query_cache_stale_hit_total") + assertTrue(after > before, + "expected a stale incremental cache hit, but query_cache_stale_hit_total stayed at ${before}") + } + + // Prove that a designed fallback phase really took the fallback path. + def checkIncrementalFallback = { String sqlText -> + if (isCloudMode()) { + checkConsistency(sqlText) + return + } + def before = sumBeMetric("query_cache_incremental_fallback_total") + checkConsistency(sqlText) + def after = sumBeMetric("query_cache_incremental_fallback_total") + assertTrue(after > before, + "expected an incremental fallback, but query_cache_incremental_fallback_total stayed at ${before}") + } + sql "set enable_nereids_distribute_planner=true" sql "set enable_query_cache=true" sql "set enable_query_cache_incremental=true" sql "set parallel_pipeline_task_num=3" sql "set enable_sql_cache=false" - sql "DROP TABLE IF EXISTS ${tableName}" + sql "DROP TABLE IF EXISTS test_query_cache_incremental" sql """ - CREATE TABLE ${tableName} ( + CREATE TABLE test_query_cache_incremental ( dt DATE, user_id INT, url STRING, @@ -81,7 +134,7 @@ suite("query_cache_incremental") { """ sql """ - INSERT INTO ${tableName} VALUES + INSERT INTO test_query_cache_incremental VALUES ('2026-01-01',1,'/a',10), ('2026-01-01',2,'/b',20), ('2026-01-02',3,'/c',30), @@ -99,6 +152,7 @@ suite("query_cache_incremental") { // Fill the cache, then serve from it. checkConsistency(querySql) + order_qt_dup_initial "${querySql}" // Simulate the hourly load pattern: only the latest partition receives new // data, so the entries of the older partitions stay valid while the hot @@ -107,34 +161,43 @@ suite("query_cache_incremental") { // exercises the forced full recompute that compacts the entry. for (int i = 1; i <= 10; i++) { sql """ - INSERT INTO ${tableName} VALUES + INSERT INTO test_query_cache_incremental VALUES ('2026-01-13',${i},'/a',${i}), ('2026-01-13',${100 + i},'/inc',${10 * i}) """ - checkConsistency(querySql) + if (i == 1) { + // The hot partition holds just two data rowsets here, far from + // both the merge-count threshold and any compaction, so on local + // storage this stale query must take the incremental path. + checkStaleIncremental(querySql) + } else { + // Later rounds may legitimately recompute in full (merge-count + // threshold, background compaction), so assert correctness only. + checkConsistency(querySql) + } } + order_qt_dup_after_appends "${querySql}" // A delete in the delta cannot be merged into the cached partial result; // the query must fall back to a full recompute and stay correct. - sql "DELETE FROM ${tableName} PARTITION p20260110 WHERE user_id = 1" - checkConsistency(querySql) + sql "DELETE FROM test_query_cache_incremental PARTITION p20260110 WHERE user_id = 1" + checkIncrementalFallback(querySql) sql """ - INSERT INTO ${tableName} VALUES + INSERT INTO test_query_cache_incremental VALUES ('2026-01-14',999,'/after-delete',1) """ checkConsistency(querySql) - - sql "DROP TABLE IF EXISTS ${tableName}" + order_qt_dup_final "${querySql}" // A UNIQUE merge-on-write table takes the incremental path as long as the // loads only append new keys (the delete bitmap of the delta window stays // empty); a load that rewrites a pre-existing key falls back to one full // recompute and re-bases the entry, after which pure appends are // incremental again. Results must stay correct in every phase. - sql "DROP TABLE IF EXISTS ${uniqueTableName}" + sql "DROP TABLE IF EXISTS test_query_cache_incremental_mow" sql """ - CREATE TABLE ${uniqueTableName} ( + CREATE TABLE test_query_cache_incremental_mow ( dt DATE, user_id INT, cost BIGINT @@ -155,29 +218,39 @@ suite("query_cache_incremental") { """ def uniqueQuerySql = """ SELECT dt, SUM(cost) AS total_cost - FROM ${uniqueTableName} + FROM test_query_cache_incremental_mow GROUP BY dt """ sql """ - INSERT INTO ${uniqueTableName} VALUES + INSERT INTO test_query_cache_incremental_mow VALUES ('2026-01-01',1,10), ('2026-01-02',2,20), ('2026-01-06',3,30) """ checkConsistency(uniqueQuerySql) + order_qt_mow_initial "${uniqueQuerySql}" // The hourly-append pattern on a primary-key table: every load only adds - // brand-new keys, so the stale entry merges incrementally. + // brand-new keys, so the stale entry merges incrementally. The first + // round is asserted through the metric (two data rowsets, nothing can + // interfere); later rounds approach the compaction threshold, so they + // assert correctness only. for (int i = 1; i <= 3; i++) { - sql "INSERT INTO ${uniqueTableName} VALUES ('2026-01-06',${100 + i},${i})" - checkConsistency(uniqueQuerySql) + sql "INSERT INTO test_query_cache_incremental_mow VALUES ('2026-01-06',${100 + i},${i})" + if (i == 1) { + checkStaleIncremental(uniqueQuerySql) + } else { + checkConsistency(uniqueQuerySql) + } } // A backfill rewrites history through the delete bitmap: user_id 1 gets a - // new cost, which forces one full recompute that re-bases the entry. - sql "INSERT INTO ${uniqueTableName} VALUES ('2026-01-01',1,100)" + // new cost, which must fall back to one full recompute that re-bases the + // entry. + sql "INSERT INTO test_query_cache_incremental_mow VALUES ('2026-01-01',1,100)" + checkIncrementalFallback(uniqueQuerySql) + // After the re-base, pure appends take the incremental path again (not + // asserted through the metric: by now enough rowsets piled up that a + // background compaction could legitimately force a full recompute). + sql "INSERT INTO test_query_cache_incremental_mow VALUES ('2026-01-06',200,7)" checkConsistency(uniqueQuerySql) - // After the re-base, pure appends take the incremental path again. - sql "INSERT INTO ${uniqueTableName} VALUES ('2026-01-06',200,7)" - checkConsistency(uniqueQuerySql) - - sql "DROP TABLE IF EXISTS ${uniqueTableName}" + order_qt_mow_final "${uniqueQuerySql}" } From ba09543cf7103685639c5c9b5c3364ffadde0c30 Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Wed, 15 Jul 2026 19:40:53 +0800 Subject: [PATCH 3/9] [fix](query-cache) Fail loudly on absent cache runtime and drop dead version plumbing Address the second round of review comments: - replace the silent lazy creation at the scan node and the degrade-to-pass-through branch at the cache source with InternalError: a missing runtime means a malformed plan shape, and silently continuing could drop data on a HIT decision - drop OlapScanner::Params::start_version and its plumbing: the pre-captured delta read source already determines what gets read, and version.first has no behavioral consumer on the BE read path - pass the fragment's QueryCacheRuntime member directly instead of a local copy - cover both fail-loud paths with unit tests --- .../exec/operator/cache_source_operator.cpp | 14 ++- be/src/exec/operator/olap_scan_operator.cpp | 10 +- .../pipeline/pipeline_fragment_context.cpp | 16 ++- be/src/exec/scan/olap_scanner.cpp | 2 +- be/src/exec/scan/olap_scanner.h | 4 - .../operator/query_cache_operator_test.cpp | 43 ++++--- .../query_cache_fragment_context_test.cpp | 119 ++++++++++++++++++ 7 files changed, 164 insertions(+), 44 deletions(-) create mode 100644 be/test/exec/pipeline/query_cache_fragment_context_test.cpp diff --git a/be/src/exec/operator/cache_source_operator.cpp b/be/src/exec/operator/cache_source_operator.cpp index 7182868d950a0e..d65d55b7a13f68 100644 --- a/be/src/exec/operator/cache_source_operator.cpp +++ b/be/src/exec/operator/cache_source_operator.cpp @@ -74,12 +74,14 @@ Status CacheSourceLocalState::init(RuntimeState* state, LocalStateInfo& info) { // operator emits the cached blocks) -- whatever their init order is, and // even if the entry gets evicted in between (the decision pins it). if (parent._query_cache_runtime == nullptr) { - // Should not happen: the fragment context always creates the runtime - // together with this operator. Degrade to an uncached scan. - LOG(WARNING) << "query cache runtime is absent, node id " << cache_param.node_id; - _need_insert_cache = false; - custom_profile()->add_info_string("HitCache", "0"); - return Status::OK(); + // The fragment context always creates the runtime together with this + // operator. Degrading to a pass-through here would silently drop data + // if the paired scan operator still made a HIT decision (it skips + // scanning while nothing emits the entry), so a broken setup must + // fail loudly, mirroring the scan side. + return Status::InternalError( + "query cache runtime is absent at the cache source, node_id={}", + cache_param.node_id); } _global_cache = parent._query_cache_runtime->cache(); _cache_decision = parent._query_cache_runtime->get_or_make_decision(scan_ranges); diff --git a/be/src/exec/operator/olap_scan_operator.cpp b/be/src/exec/operator/olap_scan_operator.cpp index 7a927d1de63b1c..7731403a1f96bc 100644 --- a/be/src/exec/operator/olap_scan_operator.cpp +++ b/be/src/exec/operator/olap_scan_operator.cpp @@ -698,13 +698,12 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { bool read_row_binlog = p._olap_scan_node.__isset.read_row_binlog && p._olap_scan_node.read_row_binlog; bool has_tso_predicate = _scan_ranges[0]->__isset.start_tso || _scan_ranges[0]->__isset.end_tso; - // Query cache incremental merge: the read sources only cover the delta - // versions (cached_version, current_version], see prepare(). + // Query cache incremental merge: the read sources captured in prepare() + // only cover the delta versions (cached_version, current_version], which + // is all the scanners need; no version plumbing is required here. const bool cache_incremental = _query_cache_decision != nullptr && _query_cache_decision->mode == QueryCacheInstanceDecision::Mode::INCREMENTAL; - const int64_t cache_start_version = - cache_incremental ? _query_cache_decision->cached_version + 1 : 0; // The flag of preagg's meaning is whether return pre agg data(or partial agg data) // PreAgg ON: The storage layer returns partially aggregated data without additional processing. (Fast data reading) @@ -714,7 +713,7 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { // PreAgg OFF: The storage layer must complete pre-aggregation and return fully aggregated data. (Slow data reading) // Incremental merge deltas are small by construction, so the parallel // scanner builder (which redistributes rowsets by size) is pointless for - // them and would lose the delta version range; use plain scanners instead. + // them; use plain scanners instead. if (enable_parallel_scan && !cache_incremental && !p._should_run_serial && p._push_down_agg_type == TPushAggOp::NONE && (_storage_no_merge() || p._olap_scan_node.is_preaggregation) @@ -827,7 +826,6 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { palo_scan_range.__isset.end_tso ? std::make_optional(palo_scan_range.end_tso) : std::nullopt, - cache_start_version, }); RETURN_IF_ERROR(scanner->init(state(), _conjuncts)); scanners->push_back(std::move(scanner)); diff --git a/be/src/exec/pipeline/pipeline_fragment_context.cpp b/be/src/exec/pipeline/pipeline_fragment_context.cpp index 45ad8366180258..def2cf473aea77 100644 --- a/be/src/exec/pipeline/pipeline_fragment_context.cpp +++ b/be/src/exec/pipeline/pipeline_fragment_context.cpp @@ -1519,11 +1519,18 @@ Status PipelineFragmentContext::_create_operator(ObjectPool* pool, const TPlanNo bool fe_with_old_version = false; switch (tnode.node_type) { case TPlanNodeType::OLAP_SCAN_NODE: { - std::shared_ptr query_cache_runtime; if (enable_query_cache) { if (_query_cache_runtime == nullptr) { - _query_cache_runtime = - std::make_shared(_params.fragment.query_cache_param); + // The plan tree is built in pre-order and the cache source + // sits above the scan, so the runtime it created must already + // exist here. Running the scan with its own runtime instead + // would silently drop data on a HIT (the scan skips scanning + // while no cache source emits the entry), so a malformed plan + // shape must fail loudly. + return Status::InternalError( + "query cache runtime is absent at the scan node, node_id={}, " + "cache node_id={}", + tnode.node_id, _params.fragment.query_cache_param.node_id); } if (tnode.olap_scan_node.__isset.read_row_binlog && tnode.olap_scan_node.read_row_binlog) { @@ -1531,12 +1538,11 @@ Status PipelineFragmentContext::_create_operator(ObjectPool* pool, const TPlanNo // neither serve nor fill the query cache. _query_cache_runtime->disable_for_binlog_scan(); } - query_cache_runtime = _query_cache_runtime; } op = std::make_shared( pool, tnode, next_operator_id(), descs, _num_instances, enable_query_cache ? _params.fragment.query_cache_param : TQueryCacheParam {}, - std::move(query_cache_runtime)); + enable_query_cache ? _query_cache_runtime : nullptr); RETURN_IF_ERROR(cur_pipe->add_operator(op, _parallel_instances)); fe_with_old_version = !tnode.__isset.is_serial_operator; break; diff --git a/be/src/exec/scan/olap_scanner.cpp b/be/src/exec/scan/olap_scanner.cpp index 894e7253c16266..a9ef277c373002 100644 --- a/be/src/exec/scan/olap_scanner.cpp +++ b/be/src/exec/scan/olap_scanner.cpp @@ -80,7 +80,7 @@ OlapScanner::OlapScanner(ScanLocalStateBase* parent, OlapScanner::Params&& param .reader_type = params.read_row_binlog ? ReaderType::READER_BINLOG : ReaderType::READER_QUERY, .aggregation = params.aggregation, - .version = {params.start_version, params.version}, + .version = {0, params.version}, .start_key {}, .end_key {}, .predicates {}, diff --git a/be/src/exec/scan/olap_scanner.h b/be/src/exec/scan/olap_scanner.h index 268165704ff770..eb8f8dee4795bf 100644 --- a/be/src/exec/scan/olap_scanner.h +++ b/be/src/exec/scan/olap_scanner.h @@ -74,10 +74,6 @@ class OlapScanner : public Scanner { TBinlogScanType::type binlog_scan_type = TBinlogScanType::NONE; std::optional start_tso; std::optional end_tso; - // Non-zero only for query cache incremental merge: read the delta - // rowsets in [start_version, version] instead of the full snapshot - // [0, version]. The read_source is pre-captured accordingly. - int64_t start_version = 0; }; OlapScanner(ScanLocalStateBase* parent, Params&& params); diff --git a/be/test/exec/operator/query_cache_operator_test.cpp b/be/test/exec/operator/query_cache_operator_test.cpp index 306f624d84ba0c..db62b44a5e8012 100644 --- a/be/test/exec/operator/query_cache_operator_test.cpp +++ b/be/test/exec/operator/query_cache_operator_test.cpp @@ -70,6 +70,10 @@ struct QueryCacheOperatorTest : public ::testing::Test { // `state` (local states) and `source`/`sink` (whose QueryCacheRuntime // owns the decisions that pin cache entries) would otherwise outlive // the cache and release their handles into a destroyed QueryCache. + // Local states built manually and never moved into `state` are + // released here too, for the same reason. + source_local_state_uptr.reset(); + sink_local_state_uptr.reset(); state.reset(); source.reset(); sink.reset(); @@ -652,7 +656,7 @@ TEST_F(QueryCacheOperatorTest, test_incremental_fallback_reason_in_profile) { EXPECT_EQ(*reason, "tablet not found"); } -TEST_F(QueryCacheOperatorTest, test_missing_runtime_degrades_to_uncached) { +TEST_F(QueryCacheOperatorTest, test_missing_runtime_fails_init) { sink = std::make_unique(); source = std::make_unique(); EXPECT_TRUE(source->set_child(child_op)); @@ -667,28 +671,23 @@ TEST_F(QueryCacheOperatorTest, test_missing_runtime_degrades_to_uncached) { cache_param.entry_max_bytes = 1024 * 1024; cache_param.entry_max_rows = 1000; - // No runtime injected: init degrades to an uncached scan, data passes - // through and nothing is written back. + // No runtime injected: a pass-through here could silently drop data if + // the paired scan still made a HIT decision, so init must fail loudly. source->_cache_param = cache_param; - create_local_state(); - - EXPECT_FALSE(source_local_state->_need_insert_cache); - - { - auto block = ColumnHelper::create_block({1, 2, 3}); - auto st = sink->sink(state.get(), &block, true); - EXPECT_TRUE(st.ok()) << st.msg(); - } - { - Block block; - bool eos = false; - auto st = source->get_block(state.get(), &block, &eos); - EXPECT_TRUE(st.ok()) << st.msg(); - EXPECT_TRUE(eos); - EXPECT_EQ(block.rows(), 3); - EXPECT_TRUE(ColumnHelper::block_equal( - block, ColumnHelper::create_block({1, 2, 3}))); - } + shared_state = sink->create_shared_state(); + source_local_state_uptr = CacheSourceLocalState::create_unique(state.get(), source.get()); + source_local_state = source_local_state_uptr.get(); + source_local_state->_global_cache = query_cache; + LocalStateInfo info {.parent_profile = &profile, + .scan_ranges = scan_ranges, + .shared_state = shared_state.get(), + .shared_state_map = {}, + .task_idx = 0}; + Status st = source_local_state_uptr->init(state.get(), info); + EXPECT_FALSE(st.ok()); + EXPECT_TRUE(st.to_string().find("query cache runtime is absent at the cache source") != + std::string::npos) + << st.to_string(); EXPECT_EQ(query_cache->get_element_count(), 0); } diff --git a/be/test/exec/pipeline/query_cache_fragment_context_test.cpp b/be/test/exec/pipeline/query_cache_fragment_context_test.cpp new file mode 100644 index 00000000000000..f00fcc9735d263 --- /dev/null +++ b/be/test/exec/pipeline/query_cache_fragment_context_test.cpp @@ -0,0 +1,119 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Reaches the private _create_operator directly: constructing a full prepare() +// payload for one switch branch would couple the test to every unrelated +// prepare step. BE UT compiles with -fno-access-control (see be/test/AGENTS.md), +// so no access hack is needed. + +#include +#include +#include +#include +#include + +#include + +#include "exec/pipeline/pipeline.h" +#include "exec/pipeline/pipeline_fragment_context.h" +#include "runtime/exec_env.h" +#include "runtime/query_context.h" + +namespace doris { + +static void empty_finish_function(RuntimeState*, Status*) {} + +// The plan tree is built in pre-order, so when the query cache is enabled the +// cache source (above the scan) must have created the shared QueryCacheRuntime +// before the scan node is reached. A missing runtime means FE sent a malformed +// plan shape; silently creating one here would let a HIT decision skip the +// scan while no cache source emits the entry, i.e. drop data, so the operator +// factory must fail loudly instead. +class QueryCacheFragmentContextTest : public testing::Test { +protected: + void SetUp() override { + TQueryOptions query_options; + TNetworkAddress fe_address; + fe_address.hostname = "127.0.0.1"; + fe_address.port = 8060; + _query_ctx = + QueryContext::create(_query_id, ExecEnv::GetInstance(), query_options, fe_address, + true, fe_address, QuerySource::INTERNAL_FRONTEND); + + TQueryCacheParam cache_param; + cache_param.__set_node_id(0); + cache_param.__set_digest("test_digest"); + _params.fragment.__set_query_cache_param(cache_param); + _context = std::make_shared( + _query_id, _params, _query_ctx, ExecEnv::GetInstance(), empty_finish_function); + } + + Status create_scan_operator(const TPlanNode& tnode) { + ObjectPool pool; + DescriptorTbl descs; + OperatorPtr op; + OperatorPtr cache_op; + PipelinePtr pipe = std::make_shared(0, 1, 1); + return _context->_create_operator(&pool, tnode, descs, op, pipe, /*parent_idx=*/-1, + /*child_idx=*/0, + /*followed_by_shuffled_operator=*/false, + /*require_bucket_distribution=*/false, cache_op); + } + + TUniqueId _query_id; + TPipelineFragmentParams _params; + std::shared_ptr _query_ctx; + std::shared_ptr _context; +}; + +TEST_F(QueryCacheFragmentContextTest, scan_without_runtime_fails_loudly) { + TPlanNode tnode; + tnode.__set_node_type(TPlanNodeType::OLAP_SCAN_NODE); + tnode.__set_node_id(7); + tnode.__set_num_children(0); + + ASSERT_EQ(_context->_query_cache_runtime, nullptr); + Status st = create_scan_operator(tnode); + EXPECT_FALSE(st.ok()); + // The full message pins the scan-side wording and the argument order + // (tnode.node_id first, then the FE-designated cache node id). + EXPECT_TRUE(st.to_string().find("query cache runtime is absent at the scan node, " + "node_id=7, cache node_id=0") != std::string::npos) + << st.to_string(); +} + +TEST_F(QueryCacheFragmentContextTest, binlog_scan_without_runtime_fails_before_binlog_handling) { + // The runtime check comes first: a row-binlog scan with a broken setup + // must fail the same way instead of dereferencing the missing runtime in + // disable_for_binlog_scan(). + TPlanNode tnode; + tnode.__set_node_type(TPlanNodeType::OLAP_SCAN_NODE); + tnode.__set_node_id(7); + tnode.__set_num_children(0); + TOlapScanNode olap_scan_node; + olap_scan_node.__set_read_row_binlog(true); + tnode.__set_olap_scan_node(olap_scan_node); + + ASSERT_EQ(_context->_query_cache_runtime, nullptr); + Status st = create_scan_operator(tnode); + EXPECT_FALSE(st.ok()); + EXPECT_TRUE(st.to_string().find("query cache runtime is absent at the scan node, " + "node_id=7, cache node_id=0") != std::string::npos) + << st.to_string(); +} + +} // namespace doris From 8d4e3c39be937d383880c07c10a01e031f2e1f31 Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Thu, 16 Jul 2026 19:47:58 +0800 Subject: [PATCH 4/9] [fix](query-cache) Reject partially captured delta windows The quiet capture returns whatever prefix of the version path it walked before a mid-window miss (it only comes back empty when the window start itself is gone), so the empty-splits guard alone could let a partial delta through: the query would then serve data only up to the prefix end yet write the entry back stamped with the full requested version, and later exact hits would silently miss the tail versions. Verify that the captured splits cover the whole (cached_version, current_version] window by checking both endpoints; the path is contiguous by construction, so the two checks pin the entire window and a partial prefix now falls back to a full recompute. The new fallback_on_partial_capture test builds exactly that scenario: rowsets {0,50} and {51,80} with a cached version 50 entry and a query at version 100. --- be/src/runtime/query_cache/query_cache.cpp | 16 +++++++++++++-- be/test/exec/pipeline/query_cache_test.cpp | 24 ++++++++++++++++++---- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/be/src/runtime/query_cache/query_cache.cpp b/be/src/runtime/query_cache/query_cache.cpp index 98112f3854f48b..64456862929a90 100644 --- a/be/src/runtime/query_cache/query_cache.cpp +++ b/be/src/runtime/query_cache/query_cache.cpp @@ -290,8 +290,9 @@ bool QueryCacheRuntime::_capture_tablet_delta(int64_t tablet_id, int64_t cached_ // quiet: on a version-graph miss (the delta merged away by compaction, an // expected per-query situation) this option makes the capture API neither - // log nor error; it returns an EMPTY read source instead, so emptiness is - // checked below as the failure signal. + // log nor error; it returns whatever PREFIX of the path it walked before + // the miss (empty when the window start itself is gone), so emptiness AND + // window coverage are both checked below as the failure signals. auto source_res = tablet->capture_read_source({cached_version + 1, decision->current_version}, {.quiet = true}); if (!source_res) { @@ -309,6 +310,17 @@ bool QueryCacheRuntime::_capture_tablet_delta(int64_t tablet_id, int64_t cached_ decision->incremental_fallback_reason = "delta versions not capturable"; return false; } + // A quiet capture broken mid-window (e.g. a replica whose local view ends + // short of current_version) yields a partial prefix, and treating it as + // INCREMENTAL would silently drop the tail rows and poison the entry on + // write-back. The path is contiguous by construction, so checking both + // endpoints pins the whole (cached_version, current_version] window. + if (read_source->rs_splits.front().rs_reader->rowset()->start_version() != cached_version + 1 || + read_source->rs_splits.back().rs_reader->rowset()->end_version() != + decision->current_version) { + decision->incremental_fallback_reason = "delta versions not capturable"; + return false; + } read_source->fill_delete_predicates(); if (!read_source->delete_predicates.empty()) { // A delete in the delta logically removes rows that are already folded diff --git a/be/test/exec/pipeline/query_cache_test.cpp b/be/test/exec/pipeline/query_cache_test.cpp index 89895ab06f2db1..4b10952cd98853 100644 --- a/be/test/exec/pipeline/query_cache_test.cpp +++ b/be/test/exec/pipeline/query_cache_test.cpp @@ -827,7 +827,8 @@ TEST_F(QueryCacheIncrementalTest, mow_irrelevant_bitmap_entries_ignored) { TEST_F(QueryCacheIncrementalTest, fallback_on_version_gap) { // The whole history lives in one compacted rowset [0, 100]: no version - // path can serve (50, 100] alone, so capture fails and we fall back. + // path can serve (50, 100] alone, so the capture comes back empty and we + // fall back. create_tablet(TKeysType::DUP_KEYS, false, {{0, 100}}); auto decision = make_stale_decision(); EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); @@ -837,9 +838,10 @@ TEST_F(QueryCacheIncrementalTest, fallback_on_version_gap) { TEST_F(QueryCacheIncrementalTest, fallback_on_capture_error) { // Non-pathfinding capture failures surface as real errors even under the - // quiet option (a pathfinding miss instead returns an empty read source, - // see fallback_on_version_gap). The storage debug point simulates such a - // failure, e.g. a rowset object missing for a found path. + // quiet option (a pathfinding miss instead returns an empty or partial + // read source, see fallback_on_version_gap and fallback_on_partial_capture). + // The storage debug point simulates such a failure, e.g. a rowset object + // missing for a found path. create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 100}}); config::enable_debug_points = true; DebugPoints::instance()->add_with_params("Tablet::capture_consistent_versions.inject_failure", @@ -862,4 +864,18 @@ TEST_F(QueryCacheIncrementalTest, fallback_on_delete_predicate) { EXPECT_EQ(decision->incremental_fallback_reason, "delta contains delete predicates"); } +TEST_F(QueryCacheIncrementalTest, fallback_on_partial_capture) { + // The quiet capture returns whatever prefix of the version path it could + // walk: with rowsets {0,50},{51,80} and a query at version 100 (a replica + // whose local view ends short of the queried version), the (50,100] + // window walks up to 80 and stops, yielding a non-empty partial prefix. + // Treating it as INCREMENTAL would silently drop the (80,100] rows, so + // the endpoint coverage check must reject it. + create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 80}}); + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(decision->key_valid); + EXPECT_EQ(decision->incremental_fallback_reason, "delta versions not capturable"); +} + } // namespace doris From b73ecf779463a3abee4d71c898397981e5fde932 Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Fri, 17 Jul 2026 00:24:38 +0800 Subject: [PATCH 5/9] [fix](query-cache) Permute cached blocks before merging and shrink the decision lock scope Address the second review round on the incremental merge: Cached-block emission: merging a cached block into the reused output block and permuting afterwards breaks whenever the block keeps its schema between pulls: the second cached block then merges positionally into already-permuted columns, a type error for heterogeneous slots and silently misplaced data for same-typed ones. The cached block is now reordered to the query's slot order through a zero-copy column view BEFORE merging, so every merge is order-aligned under both block shapes. Today the broken shape stays latent by accident: this operator is constructed without a plan node, so its own row descriptor reports zero materialized slots and clear_column_data() wipes the whole block every pull; the pipeline driver's clears are schema-keeping though, so a real row descriptor (or dropping the redundant per-operator wipe) would have armed it. Decision building: the fragment-wide mutex no longer spans _make_decision(). Candidates are built outside the lock and published through an emplace race (losers adopt the winner and release their duplicate candidate outside the lock; the winner settles the stale-hit/fallback metrics exactly once), so unrelated instances' keys stop serializing behind one slow decision. The merge-on-write history-rewrite scan also stops walking the whole delete-bitmap map under its shared lock: it hops between (rowset, segment) groups with lower_bound/upper_bound, bounding the writer-excluding hold by the in-window entries plus one logarithmic hop per group. Comparisons stay in the uint64 key domain, so the hop makes forward progress for arbitrary map contents, like the plain walk it replaces. Tests: two operator tests drive the multi-block reordered emission and the reordered incremental write-back through one schema-carrying reused block (they crash on the pre-fix code); three racer tests pin the publish race via a pre-publish sync point (one shared decision, metrics settled once, the winner's captured read source intact); negative assertions pin every deliberate-MISS path as metric-free; the bitmap pins cover an out-of-int64-range version alone in its group (the int64-cast scan livelocks there) and an in-window rewrite on a later segment of a hopped rowset. --- .../exec/operator/cache_source_operator.cpp | 33 ++- be/src/runtime/query_cache/query_cache.cpp | 90 ++++++-- .../operator/query_cache_operator_test.cpp | 206 +++++++++++++++++ be/test/exec/pipeline/query_cache_test.cpp | 207 +++++++++++++++++- 4 files changed, 501 insertions(+), 35 deletions(-) diff --git a/be/src/exec/operator/cache_source_operator.cpp b/be/src/exec/operator/cache_source_operator.cpp index d65d55b7a13f68..ecbf67c3136144 100644 --- a/be/src/exec/operator/cache_source_operator.cpp +++ b/be/src/exec/operator/cache_source_operator.cpp @@ -203,22 +203,37 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b // the delta scan -- a possible future latency optimization. const auto& hit_cache_block = local_state._hit_cache_results->at(local_state._hit_cache_pos++); + // Reorder the cached block to this query's slot order BEFORE merging + // (a zero-copy view: inserting reuses the COW column pointers). + // Merging first and permuting afterwards is an order trap on the + // second cached block whenever the reused output block keeps its + // schema between pulls: a cached-order block then merges positionally + // into query-order columns -- a type error for heterogeneous slots, + // silently misplaced data for same-typed ones. Today the trap stays + // latent by accident: this operator is built without a plan node, so + // its own _row_descriptor reports zero materialized slots and the + // clear_column_data() above wipes the whole block every pull. The + // pipeline driver's clears are schema-keeping though, so giving this + // operator a real row descriptor (or dropping the redundant-looking + // wipe above) would arm it; reordering the source keeps the merge + // order-aligned under both block shapes. + Block reordered_cache_block; + const Block* cache_block_to_merge = hit_cache_block.get(); + if (!local_state._hit_cache_column_orders.empty()) { + for (auto loc : local_state._hit_cache_column_orders) { + reordered_cache_block.insert(hit_cache_block->get_by_position(loc)); + } + cache_block_to_merge = &reordered_cache_block; + } if (need_clone_empty) { - *block = hit_cache_block->clone_empty(); + *block = cache_block_to_merge->clone_empty(); } { ScopedMutableBlock scoped_mutable_block(block); auto& mutable_block = scoped_mutable_block.mutable_block(); - RETURN_IF_ERROR(mutable_block.merge(*hit_cache_block)); + RETURN_IF_ERROR(mutable_block.merge(*cache_block_to_merge)); scoped_mutable_block.restore(); } - if (!local_state._hit_cache_column_orders.empty()) { - auto datas = block->get_columns_with_type_and_name(); - block->clear(); - for (auto loc : local_state._hit_cache_column_orders) { - block->insert(datas[loc]); - } - } if (local_state._is_incremental && local_state._need_insert_cache) { // The emitted block is already reordered to this query's slot // orders; keep a copy so the written-back entry holds diff --git a/be/src/runtime/query_cache/query_cache.cpp b/be/src/runtime/query_cache/query_cache.cpp index 64456862929a90..2946b8a51cc87d 100644 --- a/be/src/runtime/query_cache/query_cache.cpp +++ b/be/src/runtime/query_cache/query_cache.cpp @@ -17,9 +17,12 @@ #include "runtime/query_cache/query_cache.h" +#include + #include "cloud/config.h" #include "common/logging.h" #include "common/metrics/doris_metrics.h" +#include "cpp/sync_point.h" #include "storage/olap_common.h" #include "storage/rowset/rowset.h" #include "storage/rowset/rowset_reader.h" @@ -143,27 +146,45 @@ std::shared_ptr QueryCacheRuntime::get_or_make_decis return _invalid_decision; } - // The lock also serializes decision making of different instances of this - // fragment. That is acceptable in the common case: a decision only touches - // tablet metadata (no data IO), so it finishes in microseconds to - // milliseconds. On very wide fragments (hundreds of tablets per instance) - // combined with tablet header lock contention (e.g. compaction meta - // commits) this serialization can stretch the fragment init tail; if that - // ever shows up in profiles, move the capture out of the lock and let the - // losing racer drop its duplicate decision. - std::lock_guard lock(_lock); - auto it = _decisions.find(cache_key); - if (it != _decisions.end()) { - return it->second; + { + std::lock_guard lock(_lock); + auto it = _decisions.find(cache_key); + if (it != _decisions.end()) { + return it->second; + } } + // Build the candidate outside the lock: a stale-entry decision walks + // tablet metadata under the header lock and, on merge-on-write tables, + // scans the delete bitmap, so its cost grows with the tablets per + // instance and the bitmap size. This runtime is fragment-wide, so a + // single lock-scoped build would serialize even unrelated instances' + // keys behind one slow decision. Racing callers of the same instance + // build duplicate candidates; the loser adopts the winner's decision + // below and its own candidate releases with the shared_ptr (the entry + // pin and any captured delta read sources go with it), which only costs + // a redundant metadata pass. The metrics are settled once, by the + // winner, after publication. auto decision = std::make_shared(); decision->key_valid = true; decision->cache_key = cache_key; decision->current_version = version; _make_decision(scan_ranges, decision.get()); - _decisions.emplace(std::move(cache_key), decision); - return decision; + TEST_SYNC_POINT("QueryCacheRuntime::get_or_make_decision.before_publish"); + + std::lock_guard lock(_lock); + auto [it, inserted] = _decisions.emplace(std::move(cache_key), decision); + if (inserted) { + if (decision->mode == QueryCacheInstanceDecision::Mode::INCREMENTAL) { + DorisMetrics::instance()->query_cache_stale_hit_total->increment(1); + } else if (decision->mode == QueryCacheInstanceDecision::Mode::MISS && + !decision->incremental_fallback_reason.empty()) { + // Only count real fallbacks: an empty reason means incremental + // merge was not enabled for this query in the first place. + DorisMetrics::instance()->query_cache_incremental_fallback_total->increment(1); + } + } + return it->second; } void QueryCacheRuntime::_make_decision(const std::vector& scan_ranges, @@ -196,19 +217,15 @@ void QueryCacheRuntime::_make_decision(const std::vector& scan if (_try_prepare_incremental(scan_ranges, decision)) { decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; - DorisMetrics::instance()->query_cache_stale_hit_total->increment(1); return; } // Stale entry not reusable: drop the pin and fall back to a full scan. + // The stale-hit/fallback metrics are settled by the caller once the + // winning candidate is published, so a losing racer cannot double-count. decision->_delta_read_sources.clear(); decision->handle = QueryCacheHandle(); decision->mode = QueryCacheInstanceDecision::Mode::MISS; - if (!decision->incremental_fallback_reason.empty()) { - // Only count real fallbacks: an empty reason means incremental merge - // was not enabled for this query in the first place. - DorisMetrics::instance()->query_cache_incremental_fallback_total->increment(1); - } } bool QueryCacheRuntime::_try_prepare_incremental(const std::vector& scan_ranges, @@ -358,16 +375,41 @@ bool QueryCacheRuntime::_delta_rewrites_history(BaseTablet& tablet, // the version becomes visible), so this scan is race free for the window // we care about. Pending entries use DeleteBitmap::TEMP_VERSION_COMMON // (= 0) and stay below the window as well. + // + // The map is ordered by (rowset, segment, version), and most entries of + // a group lie OUTSIDE the delta window (dedup history below it, pending + // entries at 0, concurrent newer loads above it), so on a long-lived + // merge-on-write tablet the map is much larger than the window. Hop from + // group to group with lower_bound/upper_bound instead of visiting every + // entry: the shared-lock hold, which excludes bitmap writers such as + // load publish, is then bounded by the in-window entries plus one + // logarithmic hop per group instead of the full map size. + // Compare versions in the uint64 key domain (both window bounds are + // non-negative), so a hypothetical out-of-int64-range stored version can + // only take the above-window hop, whose target is strictly past the + // whole group: forward progress holds for arbitrary map contents, like + // the plain walk this replaces. + const auto window_begin = static_cast(cached_version) + 1; + const auto window_end = static_cast(current_version); const DeleteBitmap& delete_bitmap = tablet.tablet_meta()->delete_bitmap(); std::shared_lock rdlock(delete_bitmap.lock); - for (const auto& [bitmap_key, bitmap] : delete_bitmap.delete_bitmap) { - const auto version = static_cast(std::get<2>(bitmap_key)); - if (version <= cached_version || version > current_version || bitmap.isEmpty()) { + const auto& bitmap_map = delete_bitmap.delete_bitmap; + auto it = bitmap_map.begin(); + while (it != bitmap_map.end()) { + const auto& [rowset_id, segment_id, version_key] = it->first; + if (version_key < window_begin) { + it = bitmap_map.lower_bound({rowset_id, segment_id, window_begin}); + continue; + } + if (version_key > window_end) { + it = bitmap_map.upper_bound( + {rowset_id, segment_id, std::numeric_limits::max()}); continue; } - if (!delta_rowsets.contains(std::get<0>(bitmap_key))) { + if (!it->second.isEmpty() && !delta_rowsets.contains(rowset_id)) { return true; } + ++it; } return false; } diff --git a/be/test/exec/operator/query_cache_operator_test.cpp b/be/test/exec/operator/query_cache_operator_test.cpp index db62b44a5e8012..ad845836ba7e43 100644 --- a/be/test/exec/operator/query_cache_operator_test.cpp +++ b/be/test/exec/operator/query_cache_operator_test.cpp @@ -691,4 +691,210 @@ TEST_F(QueryCacheOperatorTest, test_missing_runtime_fails_init) { EXPECT_EQ(query_cache->get_element_count(), 0); } +TEST_F(QueryCacheOperatorTest, test_hit_cache_multi_block_reordered_slots) { + // Two normalized-equivalent queries can share one digest with different + // output slot orders, so the entry is served through a column + // permutation. With MORE THAN ONE cached block the permutation must be + // applied to each cached block before merging it into the reused output + // block: the pipeline's clear_column_data() keeps the already-permuted + // schema, so merging a cached-order block into it positionally breaks -- + // a type error for heterogeneous slots like these (BIGINT/INT swapped), + // silently misplaced data for same-typed ones. + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + auto* row_desc = new MockRowDescriptor { + {std::make_shared(), std::make_shared()}, &pool}; + child_op->_mock_row_desc.reset(row_desc); + // The mock descriptor leaves every slot id at its default; give the two + // output slots distinct ids so the slot-order mapping is meaningful. + auto& slots = static_cast(row_desc->tuple_desc_map.front())->Slots; + slots[0]->_id = 100; + slots[1]->_id = 101; + + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[100] = 10; + cache_param.output_slot_mapping[101] = 11; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + + { + // The entry was written by the sibling query whose output order was + // (INT32 slot 11, INT64 slot 10) -- the reverse of this query. + int64_t version = 0; + std::string cache_key; + EXPECT_TRUE( + QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = Block({ColumnHelper::create_column_with_name({1, 2}), + ColumnHelper::create_column_with_name({10, 20})}); + result.push_back(std::make_unique()); + *result.back() = Block({ColumnHelper::create_column_with_name({3}), + ColumnHelper::create_column_with_name({30})}); + query_cache->insert(cache_key, version, result, {11, 10}, 1); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + // In production this operator is also built without a plan node, so its + // _row_descriptor reports zero materialized slots and clear_column_data() + // wipes the block every pull -- the schema-carrying reused-block shape is + // accidentally unreachable today. Report a real slot count here to pin + // the permute-before-merge invariant against the natural cleanups (a real + // row descriptor, or removing the redundant per-operator wipe) that would + // make that shape live. + source->_row_descriptor._num_materialized_slots = 2; + create_local_state(); + + EXPECT_EQ(source_local_state->_slot_orders, (std::vector {10, 11})); + EXPECT_EQ(source_local_state->_hit_cache_column_orders, (std::vector {1, 0})); + + { + auto block = ColumnHelper::create_block({0}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + // ONE output block reused across pulls, exactly like the pipeline driver + // (PipelineTask feeds the same block every iteration and + // clear_column_data() keeps its schema): pull 2 is the shape that broke, + // because the reused block already carries the permuted schema. + Block block; + { + // First cached block, permuted to this query's order. + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_FALSE(eos); + Block expected({ColumnHelper::create_column_with_name({10, 20}), + ColumnHelper::create_column_with_name({1, 2})}); + EXPECT_TRUE(ColumnHelper::block_equal(block, expected)) << block.dump_data(); + } + { + // Second cached block through the schema-carrying reused block. + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_FALSE(eos); + Block expected({ColumnHelper::create_column_with_name({30}), + ColumnHelper::create_column_with_name({3})}); + EXPECT_TRUE(ColumnHelper::block_equal(block, expected)) << block.dump_data(); + } +} + +TEST_F(QueryCacheOperatorTest, test_incremental_reordered_write_back) { + // An INCREMENTAL merge through a permuted entry: the cached blocks are + // emitted in this query's slot order and the written-back entry must + // hold "cached + delta" under this query's slot order as one consistent + // whole, so a later exact hit through it permutes correctly again. + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + auto* row_desc = new MockRowDescriptor { + {std::make_shared(), std::make_shared()}, &pool}; + child_op->_mock_row_desc.reset(row_desc); + auto& slots = static_cast(row_desc->tuple_desc_map.front())->Slots; + slots[0]->_id = 100; + slots[1]->_id = 101; + + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[100] = 10; + cache_param.output_slot_mapping[101] = 11; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + { + // Two stale cached blocks written by the reverse-ordered sibling. + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = Block({ColumnHelper::create_column_with_name({1, 2}), + ColumnHelper::create_column_with_name({10, 20})}); + result.push_back(std::make_unique()); + *result.back() = Block({ColumnHelper::create_column_with_name({3}), + ColumnHelper::create_column_with_name({30})}); + query_cache->insert(cache_key, 100, result, {11, 10}, 1, /*delta_count=*/0); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + { + auto decision = std::make_shared(); + decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; + decision->key_valid = true; + decision->cache_key = cache_key; + decision->current_version = version; + decision->cached_version = 100; + decision->cached_delta_count = 0; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &decision->handle)); + source->_query_cache_runtime->inject_decision_for_test(cache_key, decision); + } + // Pin the schema-carrying reused-block shape; see the rationale in + // test_hit_cache_multi_block_reordered_slots. + source->_row_descriptor._num_materialized_slots = 2; + create_local_state(); + + EXPECT_TRUE(source_local_state->_is_incremental); + EXPECT_TRUE(source_local_state->_need_insert_cache); + EXPECT_EQ(source_local_state->_hit_cache_column_orders, (std::vector {1, 0})); + + { + // The delta partial result arrives in THIS query's slot order. + auto block = Block({ColumnHelper::create_column_with_name({40}), + ColumnHelper::create_column_with_name({4})}); + auto st = sink->sink(state.get(), &block, true); + EXPECT_TRUE(st.ok()) << st.msg(); + } + // ONE output block reused across pulls, like the pipeline driver: the + // second cached pull must permute correctly into the schema-carrying + // reused block before its write-back copy is taken. + Block block; + for (int i = 0; i < 2; ++i) { + // Both cached blocks flow out permuted, without merge errors. + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_FALSE(eos); + } + { + // Then the delta. + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + EXPECT_EQ(block.rows(), 1); + } + + // The merged entry holds every block under THIS query's slot order. + doris::QueryCacheHandle handle; + EXPECT_TRUE(query_cache->lookup(cache_key, 114514, &handle)); + EXPECT_EQ(handle.get_cache_delta_count(), 1); + EXPECT_EQ(*handle.get_cache_slot_orders(), (std::vector {10, 11})); + const auto& cached_blocks = *handle.get_cache_result(); + EXPECT_EQ(cached_blocks.size(), 3); + Block expected0({ColumnHelper::create_column_with_name({10, 20}), + ColumnHelper::create_column_with_name({1, 2})}); + EXPECT_TRUE(ColumnHelper::block_equal(*cached_blocks[0], expected0)) + << cached_blocks[0]->dump_data(); + Block expected1({ColumnHelper::create_column_with_name({30}), + ColumnHelper::create_column_with_name({3})}); + EXPECT_TRUE(ColumnHelper::block_equal(*cached_blocks[1], expected1)) + << cached_blocks[1]->dump_data(); + Block expected2({ColumnHelper::create_column_with_name({40}), + ColumnHelper::create_column_with_name({4})}); + EXPECT_TRUE(ColumnHelper::block_equal(*cached_blocks[2], expected2)) + << cached_blocks[2]->dump_data(); +} + } // namespace doris diff --git a/be/test/exec/pipeline/query_cache_test.cpp b/be/test/exec/pipeline/query_cache_test.cpp index 4b10952cd98853..ba91148307ff8d 100644 --- a/be/test/exec/pipeline/query_cache_test.cpp +++ b/be/test/exec/pipeline/query_cache_test.cpp @@ -23,7 +23,11 @@ #include #include +#include +#include +#include #include +#include #include #include @@ -31,6 +35,7 @@ #include "common/config.h" #include "common/metrics/doris_metrics.h" #include "core/data_type/data_type_number.h" +#include "cpp/sync_point.h" #include "io/fs/local_file_system.h" #include "json2pb/json_to_pb.h" #include "storage/data_dir.h" @@ -43,6 +48,7 @@ #include "storage/tablet/tablet_meta.h" #include "testutil/column_helper.h" #include "util/debug_points.h" +#include "util/defer_op.h" #include "util/uid_util.h" namespace doris { @@ -368,11 +374,19 @@ TEST_F(QueryCacheTest, runtime_decision_invalid_key) { cache_param.tablet_to_range.insert({43, "range"}); QueryCacheRuntime runtime(cache_param, cache.get()); + int64_t stale_before = DorisMetrics::instance()->query_cache_stale_hit_total->value(); + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); auto decision = runtime.get_or_make_decision(scan_ranges); EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); EXPECT_FALSE(decision->key_valid); // Every caller shares one immutable invalid decision (and one log line). EXPECT_EQ(decision.get(), runtime.get_or_make_decision(scan_ranges).get()); + // The degraded-scan path returns before the candidate build, so it must + // settle no metrics at all. + EXPECT_EQ(DorisMetrics::instance()->query_cache_stale_hit_total->value(), stale_before); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before); } TEST_F(QueryCacheTest, runtime_decision_hit) { @@ -385,11 +399,18 @@ TEST_F(QueryCacheTest, runtime_decision_hit) { EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); insert_entry(cache.get(), cache_key, 100, 0); + int64_t stale_before = DorisMetrics::instance()->query_cache_stale_hit_total->value(); + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); QueryCacheRuntime runtime(cache_param, cache.get()); auto decision = runtime.get_or_make_decision(scan_ranges); EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::HIT); EXPECT_TRUE(decision->handle.valid()); EXPECT_EQ(decision->handle.get_cache_version(), 100); + // An exact hit is neither a stale hit nor a fallback. + EXPECT_EQ(DorisMetrics::instance()->query_cache_stale_hit_total->value(), stale_before); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before); } TEST_F(QueryCacheTest, runtime_decision_force_refresh) { @@ -403,12 +424,18 @@ TEST_F(QueryCacheTest, runtime_decision_force_refresh) { insert_entry(cache.get(), cache_key, 100, 0); cache_param.__set_force_refresh_query_cache(true); + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); QueryCacheRuntime runtime(cache_param, cache.get()); auto decision = runtime.get_or_make_decision(scan_ranges); // Recompute and write back even though a fresh entry exists. EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); EXPECT_TRUE(decision->key_valid); EXPECT_FALSE(decision->handle.valid()); + // A forced refresh is a deliberate MISS with an empty reason, not a + // fallback: the caller-side settlement must not count it. + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before); } TEST_F(QueryCacheTest, runtime_decision_binlog_scan) { @@ -421,12 +448,17 @@ TEST_F(QueryCacheTest, runtime_decision_binlog_scan) { EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); insert_entry(cache.get(), cache_key, 100, 0); + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); QueryCacheRuntime runtime(cache_param, cache.get()); runtime.disable_for_binlog_scan(); auto decision = runtime.get_or_make_decision(scan_ranges); - // A binlog scan must neither serve the cached entry nor write back. + // A binlog scan must neither serve the cached entry nor write back, and + // its deliberate MISS (empty reason) must not count as a fallback. EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); EXPECT_FALSE(decision->key_valid); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before); } TEST_F(QueryCacheTest, runtime_decision_stale_without_incremental) { @@ -440,11 +472,17 @@ TEST_F(QueryCacheTest, runtime_decision_stale_without_incremental) { insert_entry(cache.get(), cache_key, 50, 0); // allow_incremental unset: a stale entry is a plain miss (full recompute). + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); QueryCacheRuntime runtime(cache_param, cache.get()); auto decision = runtime.get_or_make_decision(scan_ranges); EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); EXPECT_TRUE(decision->key_valid); EXPECT_FALSE(decision->handle.valid()); + // Incremental merge never engaged (empty reason), so the caller-side + // settlement must not count a fallback. + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before); } TEST_F(QueryCacheTest, runtime_decision_stale_incremental_fallbacks) { @@ -600,6 +638,12 @@ class QueryCacheIncrementalTest : public testing::Test { } void TearDown() override { + // Idempotent guard: a fatal assertion between enable_processing() and + // the in-test cleanup (see the racer tests) must not leak an enabled + // SyncPoint with callbacks over dead stack variables to later tests. + auto* sp = SyncPoint::get_instance(); + sp->disable_processing(); + sp->clear_all_call_backs(); _cache.reset(); _data_dir.reset(); ExecEnv::GetInstance()->set_storage_engine(nullptr); @@ -699,6 +743,55 @@ class QueryCacheIncrementalTest : public testing::Test { return runtime.get_or_make_decision(scan_ranges); } + // Runs two get_or_make_decision callers concurrently, parked at the + // pre-publish sync point until both built a live candidate, so the + // publish genuinely races. Returns both results (same object expected). + std::pair, + std::shared_ptr> + race_two_callers(QueryCacheRuntime& runtime, const std::vector& scan_ranges) { + auto* sp = SyncPoint::get_instance(); + std::atomic arrived {0}; + sp->set_call_back("QueryCacheRuntime::get_or_make_decision.before_publish", [&](auto&&) { + arrived.fetch_add(1); + // Deadline instead of an unbounded spin: if publication ever + // stops passing through this sync point, fail the test instead + // of hanging the whole UT binary. + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + while (arrived.load() < 2 && std::chrono::steady_clock::now() < deadline) { + std::this_thread::yield(); + } + EXPECT_GE(arrived.load(), 2) << "racer barrier timed out"; + }); + sp->enable_processing(); + + std::shared_ptr d1; + std::shared_ptr d2; + std::thread t1; + std::thread t2; + // If the second thread's constructor ever throws (resource + // exhaustion), destroying the still-joinable first one would + // std::terminate() the whole UT binary; join both on every exit. + Defer join_guard {[&] { + if (t1.joinable()) { + t1.join(); + } + if (t2.joinable()) { + t2.join(); + } + }}; + t1 = std::thread([&] { d1 = runtime.get_or_make_decision(scan_ranges); }); + t2 = std::thread([&] { d2 = runtime.get_or_make_decision(scan_ranges); }); + t1.join(); + t2.join(); + sp->disable_processing(); + sp->clear_all_call_backs(); + // Guard against vacuous passes: if the production sync point is ever + // renamed or removed, the barrier never fires and the racer tests + // would silently stop exercising the two-candidate race. + EXPECT_EQ(arrived.load(), 2); + return {d1, d2}; + } + std::string _absolute_dir; StorageEngine* _engine = nullptr; std::unique_ptr _data_dir; @@ -794,7 +887,9 @@ TEST_F(QueryCacheIncrementalTest, mow_history_rewrite_falls_back) { // folded into the cached entry cannot be subtracted, so fall back. auto tablet = create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}); ASSERT_NE(tablet, nullptr); - tablet->tablet_meta()->delete_bitmap().add({rowset_id_for(0), 0, 60}, 7); + // Stamped exactly at the window's upper edge (= current_version), which + // must be inclusive. + tablet->tablet_meta()->delete_bitmap().add({rowset_id_for(0), 0, 100}, 7); int64_t fallbacks_before = DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); auto decision = make_stale_decision(); @@ -820,11 +915,37 @@ TEST_F(QueryCacheIncrementalTest, mow_irrelevant_bitmap_entries_ignored) { delete_bitmap.add({rowset_id_for(0), 0, 150}, 2); // version > current delete_bitmap.add({rowset_id_for(0), 0, 0}, 4); // pending (TEMP_VERSION_COMMON) delete_bitmap.set({rowset_id_for(0), 0, 60}, roaring::Roaring()); // empty bitmap + // An out-of-int64-range version, only producible by corrupt persisted + // meta: must take the above-window hop and stay skipped. Placed as the + // ONLY entry of its (rowset, segment) group on purpose -- the int64-cast + // predecessor of the skip-scan classified such a key as below-window and + // its lower_bound hop then landed back on the key itself, livelocking. + // An above-window sibling in the same group would mask that: its own hop + // overshoots this key so it is never classified (an in-window sibling + // would merely relocate the landing into a two-entry livelock). + delete_bitmap.add({rowset_id_for(0), 1, std::numeric_limits::max()}, 5); auto decision = make_stale_decision(); EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); EXPECT_TRUE(decision->incremental_fallback_reason.empty()); } +TEST_F(QueryCacheIncrementalTest, mow_rewrite_on_later_segment_still_detected) { + // Pins two precision properties of the bitmap skip-scan: the above-window + // hop must stay within its (rowset, segment) group -- an in-window + // rewrite on a LATER segment of the same rowset must still be seen -- + // and the window's lower edge (cached_version + 1) is inclusive. A hop + // that jumped the whole rowset would miss the segment-1 evidence and + // wrongly keep the incremental path. + auto tablet = create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}); + ASSERT_NE(tablet, nullptr); + auto& delete_bitmap = tablet->tablet_meta()->delete_bitmap(); + delete_bitmap.add({rowset_id_for(0), 0, 150}, 1); // segment 0: above window, hops + delete_bitmap.add({rowset_id_for(0), 1, 51}, 2); // segment 1: at window begin, foreign + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_EQ(decision->incremental_fallback_reason, "delta rewrites history rows"); +} + TEST_F(QueryCacheIncrementalTest, fallback_on_version_gap) { // The whole history lives in one compacted rowset [0, 100]: no version // path can serve (50, 100] alone, so the capture comes back empty and we @@ -878,4 +999,86 @@ TEST_F(QueryCacheIncrementalTest, fallback_on_partial_capture) { EXPECT_EQ(decision->incremental_fallback_reason, "delta versions not capturable"); } +TEST_F(QueryCacheIncrementalTest, concurrent_racers_share_one_decision) { + // The candidate decision is built outside the runtime lock (a stale + // merge-on-write decision may scan a large delete bitmap there), so two + // operators of the same instance can race: the sync point parks both + // threads right before publication, forcing two live candidates. Exactly + // one must win, both callers must observe the winner, and the metrics + // must be settled once. + create_tablet(TKeysType::DUP_KEYS, false, {{0, 50}, {51, 100}}); + auto scan_ranges = make_scan_ranges(kTabletId, "100"); + auto cache_param = make_cache_param(kTabletId); + cache_param.__set_allow_incremental(true); + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(_cache.get(), cache_key, 50, 0); + QueryCacheRuntime runtime(cache_param, _cache.get()); + + int64_t stale_hits_before = DorisMetrics::instance()->query_cache_stale_hit_total->value(); + auto [d1, d2] = race_two_callers(runtime, scan_ranges); + + ASSERT_NE(d1, nullptr); + EXPECT_EQ(d1.get(), d2.get()); + EXPECT_EQ(d1->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); + EXPECT_EQ(DorisMetrics::instance()->query_cache_stale_hit_total->value(), + stale_hits_before + 1); + // The winner's captured read source is intact no matter which candidate + // won: the loser's duplicate capture released with its own candidate. + auto source = d1->take_delta_read_source(kTabletId); + ASSERT_NE(source, nullptr); + EXPECT_EQ(source->rs_splits.size(), 1); +} + +TEST_F(QueryCacheIncrementalTest, concurrent_racers_on_miss_settle_no_metrics) { + // No cache entry exists at all: both racers build plain-MISS candidates + // with an empty fallback reason (incremental merge never engaged), so + // publication must settle neither metric while both callers still adopt + // one shared object. + auto scan_ranges = make_scan_ranges(kTabletId, "100"); + auto cache_param = make_cache_param(kTabletId); + cache_param.__set_allow_incremental(true); + QueryCacheRuntime runtime(cache_param, _cache.get()); + + int64_t stale_before = DorisMetrics::instance()->query_cache_stale_hit_total->value(); + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); + auto [d1, d2] = race_two_callers(runtime, scan_ranges); + + ASSERT_NE(d1, nullptr); + EXPECT_EQ(d1.get(), d2.get()); + EXPECT_EQ(d1->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_TRUE(d1->incremental_fallback_reason.empty()); + EXPECT_EQ(DorisMetrics::instance()->query_cache_stale_hit_total->value(), stale_before); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before); +} + +TEST_F(QueryCacheIncrementalTest, concurrent_racers_settle_fallback_once) { + // Both racers build a fallback candidate (an AGG-keys tablet rejects the + // incremental path with a non-empty reason): the fallback counter must + // move exactly once, by the published winner. + create_tablet(TKeysType::AGG_KEYS, false, {{0, 50}, {51, 100}}); + auto scan_ranges = make_scan_ranges(kTabletId, "100"); + auto cache_param = make_cache_param(kTabletId); + cache_param.__set_allow_incremental(true); + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + insert_entry(_cache.get(), cache_key, 50, 0); + QueryCacheRuntime runtime(cache_param, _cache.get()); + + int64_t fallback_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); + auto [d1, d2] = race_two_callers(runtime, scan_ranges); + + ASSERT_NE(d1, nullptr); + EXPECT_EQ(d1.get(), d2.get()); + EXPECT_EQ(d1->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_EQ(d1->incremental_fallback_reason, "keys type not append-only"); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallback_before + 1); +} + } // namespace doris From bdd1b13ec05274e07a3bc9263fe467b7cbf7f32b Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Fri, 17 Jul 2026 09:17:53 +0800 Subject: [PATCH 6/9] [fix](query-cache) Publish the write-back entry only on success and snapshot cached blocks zero-copy Address the third review round on the incremental merge: Entry commit: the write-back entry was published from an unconditional Defer that also ran during error unwinding. On the final delta block eos is observed BEFORE the block is merged, so a merge failure there (allocation failure surfaces as a non-OK Status through RETURN_IF_CATCH_EXCEPTION) published an entry that was missing the failed block under the current version, and a later exact hit would serve the incomplete result silently. The commit now happens only at the two explicit success exits (the empty pull that observes eos, and the exit where the pulled block was fully consumed), so every error return leaves the cache untouched and the pinned stale entry stays the served truth. This also closes the exception-unwind exit (the old Defer ran the insert inside a destructor while unwinding, which could std::terminate if the insert itself threw), and the insert is latched exactly-once, so even an out-of-contract extra poll after eos cannot replace the committed entry with an empty one. Write-back copies: replacing an entry peaked at roughly three payloads (the pinned old entry, the deep-copied write-back set, and the insert-time copy), and the LRU accounting could not see the pinned one once the same-key insert displaced it. The cached blocks are now snapshotted as zero-copy views sharing the COW column pointers of the pinned entry, and the single materialization stays in QueryCache::insert, so a replacement peaks at the old entry plus the new entry. Delta blocks were already moved and are unchanged. Tests: a new case injects a deterministic merge failure on the final delta block and asserts the query fails, nothing is published at the current version, and the stale entry is untouched; the reordered write-back case now pins the column sharing before insert, the single materialization after insert, and that an extra poll after eos keeps the committed entry intact; test_no_hit_cache1 asserts the latched flag, with the element count as the insert witness. --- .../exec/operator/cache_source_operator.cpp | 45 +++++-- be/src/exec/operator/cache_source_operator.h | 12 +- .../operator/query_cache_operator_test.cpp | 123 +++++++++++++++++- 3 files changed, 162 insertions(+), 18 deletions(-) diff --git a/be/src/exec/operator/cache_source_operator.cpp b/be/src/exec/operator/cache_source_operator.cpp index ecbf67c3136144..8a055767ed9871 100644 --- a/be/src/exec/operator/cache_source_operator.cpp +++ b/be/src/exec/operator/cache_source_operator.cpp @@ -157,15 +157,18 @@ bool CacheSourceLocalState::_account_write_back(int64_t rows, int64_t bytes) { return true; } -Status CacheSourceLocalState::_append_block_for_write_back(Block& block) { +Status CacheSourceLocalState::_append_block_for_write_back(const Block& block) { if (!_account_write_back(block.rows(), block.allocated_bytes())) { return Status::OK(); } - auto& cloned = _local_cache_blocks.emplace_back(Block::create_unique()); - cloned->swap(block.clone_empty()); - ScopedMutableBlock scoped_mutable_block(cloned.get()); - auto& mutable_block = scoped_mutable_block.mutable_block(); - RETURN_IF_ERROR(mutable_block.merge(block)); + // Zero-copy snapshot: inserting shares the COW column pointers instead + // of cloning the payload. The cached columns stay alive (and immutable) + // through the decision's entry pin until QueryCache::insert() deep-copies + // the accumulated set once. + auto& kept = _local_cache_blocks.emplace_back(Block::create_unique()); + for (const auto& column : block.get_columns_with_type_and_name()) { + kept->insert(column); + } return Status::OK(); } @@ -235,17 +238,24 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b scoped_mutable_block.restore(); } if (local_state._is_incremental && local_state._need_insert_cache) { - // The emitted block is already reordered to this query's slot - // orders; keep a copy so the written-back entry holds - // "cached + delta" under one consistent slot order. - RETURN_IF_ERROR(local_state._append_block_for_write_back(*block)); + // Snapshot the cached block (already in this query's slot order) + // for the write-back, so the new entry holds "cached + delta" + // under one consistent slot order. The snapshot shares the COW + // columns of the pinned entry, so no payload is copied before + // the insert-time materialization. + RETURN_IF_ERROR(local_state._append_block_for_write_back(*cache_block_to_merge)); } } else if (local_state._hit_cache_results != nullptr && !local_state._is_incremental) { // HIT: all cached blocks are emitted. *eos = true; } else { // MISS, or the delta phase of INCREMENTAL after the cached blocks. - Defer insert_cache([&] { + // The entry is committed only from the explicit success paths below, + // never during error unwinding: on the final delta block *eos is set + // BEFORE the block is merged, so a deferred/unconditional commit + // would publish an entry that is missing the failed block under the + // current version, and a later exact hit would silently serve it. + auto commit_entry_if_finished = [&]() { if (*eos) { local_state.custom_profile()->add_info_string( "InsertCache", std::to_string(local_state._need_insert_cache)); @@ -256,9 +266,18 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b local_state._current_query_cache_bytes, local_state._insert_delta_count); local_state._local_cache_blocks.clear(); + // Latch off so the commit is exactly-once by construction: + // a spurious re-poll after eos (no in-tree driver does + // this today) would otherwise re-publish the now-cleared, + // EMPTY block set over the entry just inserted, and later + // exact hits would silently serve an empty result. This + // is the only source operator whose eos path carries a + // global side effect, so it must not rely on the pull + // protocol never poking it again. + local_state._need_insert_cache = false; } } - }); + }; std::unique_ptr output_block; int child_idx = 0; @@ -269,6 +288,7 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b *eos = !_has_data(state) && local_state._shared_state->data_queue.is_all_finish(); if (!output_block) { + commit_entry_if_finished(); return Status::OK(); } @@ -287,6 +307,7 @@ Status CacheSourceOperatorX::get_block_impl(RuntimeState* state, Block* block, b } else { *block = std::move(*output_block); } + commit_entry_if_finished(); } local_state.reached_limit(block, eos); diff --git a/be/src/exec/operator/cache_source_operator.h b/be/src/exec/operator/cache_source_operator.h index 1d9da4ae183b18..0378638458a101 100644 --- a/be/src/exec/operator/cache_source_operator.h +++ b/be/src/exec/operator/cache_source_operator.h @@ -53,10 +53,14 @@ class CacheSourceLocalState final : public PipelineXLocalState({1, 2, 3, 4, 5}))); EXPECT_EQ(query_cache->get_element_count(), 1); } - EXPECT_TRUE(source_local_state->_need_insert_cache); + // A successful commit latches the flag off (the insert is exactly-once); + // that the query cached at all is asserted by the element count above, + // in contrast to test_no_hit_cache2 where the over-limit entry is + // dropped and the count stays 0. + EXPECT_FALSE(source_local_state->_need_insert_cache); } TEST_F(QueryCacheOperatorTest, test_no_hit_cache2) { @@ -858,7 +862,7 @@ TEST_F(QueryCacheOperatorTest, test_incremental_reordered_write_back) { } // ONE output block reused across pulls, like the pipeline driver: the // second cached pull must permute correctly into the schema-carrying - // reused block before its write-back copy is taken. + // reused block before its write-back snapshot is taken. Block block; for (int i = 0; i < 2; ++i) { // Both cached blocks flow out permuted, without merge errors. @@ -867,6 +871,18 @@ TEST_F(QueryCacheOperatorTest, test_incremental_reordered_write_back) { EXPECT_TRUE(st.ok()) << st.msg(); EXPECT_FALSE(eos); } + // The write-back snapshots are zero-copy views over the pinned entry's + // columns (in this query's order), not payload copies: the single + // materialization happens at insert time. + const auto& pinned_blocks = *source_local_state->_cache_decision->handle.get_cache_result(); + const auto& kept_blocks = source_local_state->_local_cache_blocks; + ASSERT_EQ(kept_blocks.size(), 2); + const auto* pinned_first_int64 = pinned_blocks[0]->get_by_position(1).column.get(); + EXPECT_EQ(kept_blocks[0]->get_by_position(0).column.get(), pinned_first_int64); + EXPECT_EQ(kept_blocks[0]->get_by_position(1).column.get(), + pinned_blocks[0]->get_by_position(0).column.get()); + EXPECT_EQ(kept_blocks[1]->get_by_position(0).column.get(), + pinned_blocks[1]->get_by_position(1).column.get()); { // Then the delta. bool eos = false; @@ -883,6 +899,9 @@ TEST_F(QueryCacheOperatorTest, test_incremental_reordered_write_back) { EXPECT_EQ(*handle.get_cache_slot_orders(), (std::vector {10, 11})); const auto& cached_blocks = *handle.get_cache_result(); EXPECT_EQ(cached_blocks.size(), 3); + // The insert materialized one cache-owned copy: the new entry does not + // alias the old pinned columns. + EXPECT_NE(cached_blocks[0]->get_by_position(0).column.get(), pinned_first_int64); Block expected0({ColumnHelper::create_column_with_name({10, 20}), ColumnHelper::create_column_with_name({1, 2})}); EXPECT_TRUE(ColumnHelper::block_equal(*cached_blocks[0], expected0)) @@ -895,6 +914,106 @@ TEST_F(QueryCacheOperatorTest, test_incremental_reordered_write_back) { ColumnHelper::create_column_with_name({4})}); EXPECT_TRUE(ColumnHelper::block_equal(*cached_blocks[2], expected2)) << cached_blocks[2]->dump_data(); + + { + // A spurious re-poll after eos must not re-publish: the commit is + // latched off after the insert, so with the write-back set already + // cleared the merged entry keeps its three blocks instead of being + // replaced by an EMPTY set under the same version. + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_TRUE(st.ok()) << st.msg(); + EXPECT_TRUE(eos); + doris::QueryCacheHandle repoll_handle; + EXPECT_TRUE(query_cache->lookup(cache_key, 114514, &repoll_handle)); + EXPECT_EQ(repoll_handle.get_cache_result()->size(), 3); + } +} + +TEST_F(QueryCacheOperatorTest, test_failed_final_delta_merge_publishes_nothing) { + // On the final delta block, eos is observed BEFORE the block is merged: + // if that merge fails, the entry must NOT be committed, or an incomplete + // prefix (cached blocks plus earlier deltas, missing the failed block) + // would be served as the current version by later exact hits. The + // malformed two-column final block makes the merge fail determinately. + sink = std::make_unique(); + source = std::make_unique(); + EXPECT_TRUE(source->set_child(child_op)); + child_op->_mock_row_desc.reset( + new MockRowDescriptor {{std::make_shared()}, &pool}); + TQueryCacheParam cache_param; + cache_param.node_id = 0; + cache_param.digest = "test_digest"; + cache_param.output_slot_mapping[0] = 0; + cache_param.tablet_to_range.insert({42, "test"}); + cache_param.force_refresh_query_cache = false; + cache_param.entry_max_bytes = 1024 * 1024; + cache_param.entry_max_rows = 1000; + cache_param.__set_allow_incremental(true); + + std::string cache_key; + int64_t version = 0; + EXPECT_TRUE(QueryCache::build_cache_key(scan_ranges, cache_param, &cache_key, &version).ok()); + { + CacheResult result; + result.push_back(std::make_unique()); + *result.back() = ColumnHelper::create_block({1, 2, 3}); + query_cache->insert(cache_key, 100, result, {0}, 1, /*delta_count=*/0); + } + + source->_cache_param = cache_param; + source->_query_cache_runtime = std::make_shared(cache_param, query_cache); + { + auto decision = std::make_shared(); + decision->mode = QueryCacheInstanceDecision::Mode::INCREMENTAL; + decision->key_valid = true; + decision->cache_key = cache_key; + decision->current_version = version; + decision->cached_version = 100; + decision->cached_delta_count = 0; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &decision->handle)); + source->_query_cache_runtime->inject_decision_for_test(cache_key, decision); + } + // Pin the schema-carrying reused-block shape (rationale in + // test_hit_cache_multi_block_reordered_slots): with the default empty row + // descriptor the reused block is wiped every pull and re-cloned from the + // incoming block, so the malformed final block would merge into a clone + // of itself and SUCCEED, unbinding this test from the bug it guards. + source->_row_descriptor._num_materialized_slots = 1; + create_local_state(); + EXPECT_TRUE(source_local_state->_need_insert_cache); + + { + // A good delta block, then a malformed final one. + auto good = ColumnHelper::create_block({6, 7}); + EXPECT_TRUE(sink->sink(state.get(), &good, false).ok()); + auto bad = Block({ColumnHelper::create_column_with_name({8}), + ColumnHelper::create_column_with_name({9})}); + EXPECT_TRUE(sink->sink(state.get(), &bad, true).ok()); + } + Block block; + { + // Cached block, then the good delta block. + bool eos = false; + EXPECT_TRUE(source->get_block(state.get(), &block, &eos).ok()); + EXPECT_FALSE(eos); + EXPECT_TRUE(source->get_block(state.get(), &block, &eos).ok()); + EXPECT_FALSE(eos); + } + { + // The final block: eos flips to true first, then the merge fails. + bool eos = false; + auto st = source->get_block(state.get(), &block, &eos); + EXPECT_FALSE(st.ok()); + } + + // Nothing was committed: no entry at the current version, and the stale + // entry still carries version 100 untouched. + doris::QueryCacheHandle handle; + EXPECT_FALSE(query_cache->lookup(cache_key, 114514, &handle)); + doris::QueryCacheHandle stale_handle; + EXPECT_TRUE(query_cache->lookup_any_version(cache_key, &stale_handle)); + EXPECT_EQ(stale_handle.get_cache_version(), 100); } } // namespace doris From 2033145942b320146968faa57bcff45814dc260b Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Fri, 17 Jul 2026 10:43:22 +0800 Subject: [PATCH 7/9] [fix](query-cache) Let the parallel scanner builder split incremental deltas Address the fourth review round on the incremental merge: The incremental scan bypassed ParallelScannerBuilder on the assumption that deltas are small by construction, but nothing enforces that bound: query_cache_max_incremental_merge_count limits prior write-backs, not the versions accumulated while an entry sits idle, so the first stale query after a long dormant stretch can face an arbitrarily large suffix (including cumulative-compaction rowsets wholly inside the window) and was scanning it with one scanner per tablet. The bypass is removed: the builder consumes the captured delta read sources exactly as it consumes full captures (each partial read source keeps the entire source's delete predicates and delete bitmap, and the rowset readers are cloned per scanner), so a large suffix is split by rowset/segment rows like any full scan, while a genuinely small delta still collapses to a single scanner through min_rows_per_scanner, which is the size gate the old bypass was approximating from outside. Tests: the regression suite gains a dormant-entry phase: fill the entry, land thirty loads on a single-tablet partition without touching it, then assert the first stale query still takes the incremental path and agrees with the uncached result, with parallel_scan_min_rows_per_scanner lowered so the builder really splits the delta into many scanners. --- be/src/exec/operator/olap_scan_operator.cpp | 20 +++--- .../cache/query_cache_incremental.groovy | 70 ++++++++++++++++++- 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/be/src/exec/operator/olap_scan_operator.cpp b/be/src/exec/operator/olap_scan_operator.cpp index 7731403a1f96bc..50d41d3dcc979a 100644 --- a/be/src/exec/operator/olap_scan_operator.cpp +++ b/be/src/exec/operator/olap_scan_operator.cpp @@ -698,12 +698,6 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { bool read_row_binlog = p._olap_scan_node.__isset.read_row_binlog && p._olap_scan_node.read_row_binlog; bool has_tso_predicate = _scan_ranges[0]->__isset.start_tso || _scan_ranges[0]->__isset.end_tso; - // Query cache incremental merge: the read sources captured in prepare() - // only cover the delta versions (cached_version, current_version], which - // is all the scanners need; no version plumbing is required here. - const bool cache_incremental = - _query_cache_decision != nullptr && - _query_cache_decision->mode == QueryCacheInstanceDecision::Mode::INCREMENTAL; // The flag of preagg's meaning is whether return pre agg data(or partial agg data) // PreAgg ON: The storage layer returns partially aggregated data without additional processing. (Fast data reading) @@ -711,10 +705,16 @@ Status OlapScanLocalState::_init_scanners(std::list* scanners) { // And the user send a query like select userid,count(*) from base table group by userid. // then the storage layer do not need do aggregation, it could just return the partial agg data, because the compute layer will do aggregation. // PreAgg OFF: The storage layer must complete pre-aggregation and return fully aggregated data. (Slow data reading) - // Incremental merge deltas are small by construction, so the parallel - // scanner builder (which redistributes rowsets by size) is pointless for - // them; use plain scanners instead. - if (enable_parallel_scan && !cache_incremental && !p._should_run_serial && + // Query cache incremental merge needs no special case here: the read + // sources captured in prepare() already cover exactly the delta versions + // (cached_version, current_version], and the parallel scanner builder + // consumes the captured read sources as they are. The delta is NOT small + // by construction (an entry can sit dormant for arbitrarily many versions + // before it is reused; the merge-count threshold bounds prior write-backs, + // not idle accumulation), so a large suffix is split by rowset/segment + // rows like any full scan, while a small delta naturally collapses to a + // single scanner through min_rows_per_scanner. + if (enable_parallel_scan && !p._should_run_serial && p._push_down_agg_type == TPushAggOp::NONE && (_storage_no_merge() || p._olap_scan_node.is_preaggregation) // binlog need to be read in order diff --git a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy index 1e093a28e3128d..a8c1d932069b96 100644 --- a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy +++ b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy @@ -25,7 +25,9 @@ // the incremental path really fires: the early stale queries must increase // query_cache_stale_hit_total, and the two designed fallback phases (a // delete predicate in the delta, a merge-on-write backfill that rewrites -// history) must increase query_cache_incremental_fallback_total. +// history) must increase query_cache_incremental_fallback_total. The final +// phase covers a long-dormant entry whose first reuse faces a large, +// many-rowset delta that goes through the parallel scanner builder. suite("query_cache_incremental") { def querySql = """ SELECT @@ -253,4 +255,70 @@ suite("query_cache_incremental") { sql "INSERT INTO test_query_cache_incremental_mow VALUES ('2026-01-06',200,7)" checkConsistency(uniqueQuerySql) order_qt_mow_final "${uniqueQuerySql}" + + // A cache entry can sit dormant while loads keep landing: the merge-count + // threshold bounds prior write-backs, not the versions accumulated while + // the entry is idle, so the delta of the first stale query after a long + // idle stretch can be arbitrarily large. Such a delta must flow through + // the parallel scanner builder like any full scan (split by segment rows) + // and still merge correctly with the cached blocks. enable_parallel_scan + // is on by default; the tiny per-scanner row bound forces the builder to + // really split the delta into many scanners instead of collapsing to one. + sql "set enable_parallel_scan=true" + sql "set parallel_scan_min_rows_per_scanner=16" + sql "DROP TABLE IF EXISTS test_query_cache_incremental_dormant" + sql """ + CREATE TABLE test_query_cache_incremental_dormant ( + dt DATE, + user_id INT, + url STRING, + cost BIGINT + ) + ENGINE=OLAP + DUPLICATE KEY(dt, user_id) + PARTITION BY RANGE(dt) + ( + PARTITION p20260101 VALUES LESS THAN ("2026-01-15") + ) + DISTRIBUTED BY HASH(user_id) BUCKETS 1 + PROPERTIES + ( + "replication_num" = "1" + ) + """ + def dormantQuerySql = """ + SELECT + url, + SUM(cost) AS total_cost, + COUNT(*) AS cnt + FROM test_query_cache_incremental_dormant + WHERE dt >= '2026-01-01' + AND dt < '2026-01-15' + GROUP BY url + """ + sql "INSERT INTO test_query_cache_incremental_dormant VALUES ('2026-01-01',1,'/a',10)" + // Fill the entry at the small initial version, then leave it dormant + // while 30 loads land on the single-tablet partition: a 30-rowset, + // couple-hundred-row suffix that splits into a dozen or more scanners + // under the 16-row bound. + checkConsistency(dormantQuerySql) + for (int i = 1; i <= 30; i++) { + sql """ + INSERT INTO test_query_cache_incremental_dormant VALUES + ('2026-01-02',${i},'/a',${i}), + ('2026-01-03',${i},'/b',${i}), + ('2026-01-04',${i},'/c',${i}), + ('2026-01-05',${i},'/d',${i}), + ('2026-01-06',${i},'/e',${i}), + ('2026-01-07',${i},'/f',${i}), + ('2026-01-08',${i},'/g',${i}) + """ + } + // The first query after the idle stretch must still take the incremental + // path on local storage and agree with the uncached result. The loads + // landed moments ago, so a boundary-crossing base compaction inside this + // window is as unlikely as in the first-round assertions above (an + // in-window cumulative compaction keeps the capture, and therefore the + // assertion, intact). + checkStaleIncremental(dormantQuerySql) } From b25ce1f51a340c780c1769ab7c6d8a52ffbeccac Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Fri, 17 Jul 2026 13:59:09 +0800 Subject: [PATCH 8/9] [fix](query-cache) Forward the query cache session variables to the planning master Address the fifth review round on the incremental merge: A forwarded statement is planned by the master in a fresh ConnectContext, which starts from the master's global values and then sees only what getForwardVariables() sends, and query cache normalization runs wherever the statement is planned. Only enable_query_cache_incremental was annotated needForward, so a client that enabled both switches on a follower reached the master with the incremental flag true while enable_query_cache fell back to the master's global (false by default): both planner gates then skipped cache normalization and the feature silently did nothing. The base switch is now forwarded too, which also fixes the same gap the base query cache had on its own before this PR: a session-level setting, or a SET_VAR hint, never reached the planner of a forwarded statement. Forwarding the switch alone would have left a second gap one hop away: QueryCacheNormalizer builds the cache param on the planning FE from three more session variables, so with the cache now engaged on forwarded paths a forced refresh would have been dropped and the entries would have been sized by the master's defaults. Those three (query_cache_force_refresh, query_cache_entry_max_bytes, query_cache_entry_max_rows) are forwarded as well, so every variable the query cache reads at plan time now travels with the statement. The digest is unaffected: it is salted from the variables annotated as affecting the query result, which needForward does not join. Behavior change to note for multi-FE clusters: a forwarded statement now plans with the session's query cache state instead of the master's global one. Statements are forwarded when the follower cannot read, when force_forward_all_queries or the debug point is on, and for every redirected statement, which includes each non-group-commit INSERT issued on a follower. Besides the cache itself, enable_query_cache carries the scan-side semantics it already has locally: OlapScanNode pins the fixed replica and, as upstream documents on skip_missing_version, that pinning turns the missing-version rescue off. Those now apply to forwarded statements too, exactly as they already do when the same session runs the same statement without forwarding, so the forwarded path stops being the odd one out. Three honesty fixes found while reviewing the above: The delete bitmap scan claimed the entries it reads are immutable once their version is visible, hence race free. They are not: compaction copies markers onto its output keeping the original version, and the stale-rowset aggregation re-stamps a swept range onto its end version, both at already visible versions and both on the local path. The check is still safe, but for a different reason, now written down: it is conservative (an in-window marker on a rowset outside the delta is read as a rewrite), the capture endpoint check rejects a window whose path was merged away, and what is left is the narrow non-atomic window between capture and scan that the exact-hit path shares. query_cache_write_back_total counts entries handed to the cache, not entries the cache kept: with an LRU-K admission a first-seen key on a full shard is dropped, so on a pressured BE the counter can climb while nothing lands. The metric text says so now. The dormant-entry regression phase never split its delta: the per-scanner floor defaults to 2M rows, so the 210-row delta stayed with one scanner and the phase covered none of the splitting it was written for. It now lowers the floor to the reachable minimum of 1024 rows (BE clamps the variable up to that) and grows the delta to 6000 rows across 30 rowsets, which the builder cuts into four or five scanners. Tests: SessionVariablesTest covers the forwarding contract end to end, setting all five variables on a follower-side session, sending them through getForwardVariables() and asserting a fresh master-side session receives every one of them instead of its defaults. --- be/src/common/metrics/doris_metrics.cpp | 3 +- be/src/common/metrics/doris_metrics.h | 4 ++- be/src/runtime/query_cache/query_cache.cpp | 28 ++++++++++++---- .../org/apache/doris/qe/SessionVariable.java | 19 ++++++++--- .../apache/doris/qe/SessionVariablesTest.java | 33 +++++++++++++++++++ .../cache/query_cache_incremental.groovy | 33 +++++++++---------- 6 files changed, 90 insertions(+), 30 deletions(-) diff --git a/be/src/common/metrics/doris_metrics.cpp b/be/src/common/metrics/doris_metrics.cpp index 64d542970098eb..de596407e512b3 100644 --- a/be/src/common/metrics/doris_metrics.cpp +++ b/be/src/common/metrics/doris_metrics.cpp @@ -55,7 +55,8 @@ DEFINE_COUNTER_METRIC_PROTOTYPE_3ARG(query_cache_incremental_fallback_total, Met "Query cache decisions that could have merged incrementally " "but fell back to a full recompute."); DEFINE_COUNTER_METRIC_PROTOTYPE_3ARG(query_cache_write_back_total, MetricUnit::REQUESTS, - "Query cache entries written back (full or merged)."); + "Query cache entries handed to the cache to be written back " + "(full or merged); admission may still turn one down."); DEFINE_COUNTER_METRIC_PROTOTYPE_5ARG(push_requests_success_total, MetricUnit::REQUESTS, "", push_requests_total, Labels({{"status", "SUCCESS"}})); DEFINE_COUNTER_METRIC_PROTOTYPE_5ARG(push_requests_fail_total, MetricUnit::REQUESTS, "", diff --git a/be/src/common/metrics/doris_metrics.h b/be/src/common/metrics/doris_metrics.h index ac6e993698d6ad..1679f28d775a8d 100644 --- a/be/src/common/metrics/doris_metrics.h +++ b/be/src/common/metrics/doris_metrics.h @@ -57,7 +57,9 @@ class DorisMetrics { // Query cache incremental merge (see runtime/query_cache/query_cache.h): // how many instance decisions reused a stale entry incrementally, how many // could have but fell back to a full recompute, and how many entries were - // written back. Per-query breakdown lives in the profile + // handed to the cache to be written back (the cache may still turn one + // down: its LRU-K admission only keeps a key that comes back while the + // shard is full). Per-query breakdown lives in the profile // (HitCacheStale / IncrementalFallbackReason / InsertCache). IntCounter* query_cache_stale_hit_total = nullptr; IntCounter* query_cache_incremental_fallback_total = nullptr; diff --git a/be/src/runtime/query_cache/query_cache.cpp b/be/src/runtime/query_cache/query_cache.cpp index 2946b8a51cc87d..db0abf2646a64f 100644 --- a/be/src/runtime/query_cache/query_cache.cpp +++ b/be/src/runtime/query_cache/query_cache.cpp @@ -369,12 +369,28 @@ bool QueryCacheRuntime::_delta_rewrites_history(BaseTablet& tablet, for (const auto& split : delta_source.rs_splits) { delta_rowsets.insert(split.rs_reader->rowset()->rowset_id()); } - // The tablet delete bitmap is shared with concurrent loads, but entries - // stamped with a version <= current_version are immutable once that - // version is visible (merge-on-write publishes the bitmap update before - // the version becomes visible), so this scan is race free for the window - // we care about. Pending entries use DeleteBitmap::TEMP_VERSION_COMMON - // (= 0) and stay below the window as well. + // The tablet delete bitmap is shared and it does change at versions that + // are already visible, so what keeps this scan safe is not immutability + // but the fallback it feeds. Loads add their markers before the version + // becomes visible. Compaction relocates existing markers onto its output + // rowset keeping their original version (BaseTablet::calc_compaction_ + // output_rowset_delete_bitmap); markers that matter here sit on a rowset + // at or below the cached version, and an output that swallowed such a + // rowset starts no later than it does, so the relocated marker lands + // outside the delta and is read as a rewrite: conservative, never a miss. + // (An output whose inputs all sit inside the window does belong to the + // delta, and markers on it are delta-internal, which is what we want.) The one + // rewrite that moves markers out of the window, the aggregation that + // re-stamps a stale range onto its end version (BaseTablet::agg_delete_ + // bitmap_for_stale_rowsets, run by delete_expired_stale_rowset), only + // fires once the merged versions are swept, and the capture this scan + // pairs with anchors on the window endpoints, so a window whose path is + // gone is rejected before we get here. Capture and scan are not atomic, + // so a sweep landing exactly in between (with the aggregated keys also + // already recycled) can still hide a rewrite: a known narrow race the + // fallback does not cover, shared with the baseline exact-hit path. + // Pending entries use DeleteBitmap::TEMP_VERSION_COMMON (= 0) and stay + // below the window as well. // // The map is ordered by (rowset, segment, version), and most entries of // a group lie OUTSIDE the delta window (dedup history below it, pending diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index 51d4b758d4ed20..fc972c67d3a940 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -1542,7 +1542,14 @@ public enum IgnoreSplitType { @VarAttrDef.VarAttr(name = ENABLE_HIVE_SQL_CACHE, fuzzy = false) public boolean enableHiveSqlCache = false; - @VarAttrDef.VarAttr(name = ENABLE_QUERY_CACHE, fuzzy = true) + // Forwarded because query cache normalization runs wherever the statement is + // planned: a forwarded statement is planned by the master in a fresh + // ConnectContext, which starts from the master's global value and then sees + // only what getForwardVariables() sends, so without this a session-level + // setting (or a SET_VAR hint) never reaches the planner: the cache follows + // the master's global instead, and enable_query_cache_incremental, which + // requires this switch, would arrive alone and never take effect. + @VarAttrDef.VarAttr(name = ENABLE_QUERY_CACHE, fuzzy = true, needForward = true) public boolean enableQueryCache = false; // Allow BE to reuse a stale query cache entry by scanning only the delta @@ -1575,13 +1582,17 @@ public enum IgnoreSplitType { + " or rewrites history rows. Requires enable_query_cache."}) public boolean enableQueryCacheIncremental = false; - @VarAttrDef.VarAttr(name = QUERY_CACHE_FORCE_REFRESH) + // Forwarded for the same reason as enable_query_cache: the master builds + // the query cache param when it plans a forwarded statement, so without + // this a forwarded statement would silently ignore a forced refresh and + // size the entries by the master's defaults instead of the session's. + @VarAttrDef.VarAttr(name = QUERY_CACHE_FORCE_REFRESH, needForward = true) private boolean queryCacheForceRefresh = false; - @VarAttrDef.VarAttr(name = QUERY_CACHE_ENTRY_MAX_BYTES) + @VarAttrDef.VarAttr(name = QUERY_CACHE_ENTRY_MAX_BYTES, needForward = true) private long queryCacheEntryMaxBytes = 5242880; - @VarAttrDef.VarAttr(name = QUERY_CACHE_ENTRY_MAX_ROWS) + @VarAttrDef.VarAttr(name = QUERY_CACHE_ENTRY_MAX_ROWS, needForward = true) private long queryCacheEntryMaxRows = 500000; @VarAttrDef.VarAttr(name = ENABLE_CONDITION_CACHE) diff --git a/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java b/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java index a8446efb451fee..a2d671befd634c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java @@ -78,6 +78,39 @@ public void testForwardSessionVariables() { sessionVariable.getInsertVisibleTimeoutReturnModeEnum()); } + @Test + public void testForwardQueryCacheVariables() { + // A forwarded statement is planned by the master in a fresh + // ConnectContext that only sees what getForwardVariables() sends, so + // every session variable the query cache reads at plan time must + // travel: the switch and its incremental companion (both planner gates + // and the eligibility check), and the three the cache param carries + // (a forced refresh must not be dropped, and entries must be sized by + // the session's limits, not the master's defaults). + SessionVariable follower = new SessionVariable(); + follower.setEnableQueryCache(true); + follower.setEnableQueryCacheIncremental(true); + follower.setQueryCacheForceRefresh(true); + follower.setQueryCacheEntryMaxBytes(4096); + follower.setQueryCacheEntryMaxRows(64); + Map vars = follower.getForwardVariables(); + Assertions.assertEquals("true", vars.get(SessionVariable.ENABLE_QUERY_CACHE)); + Assertions.assertEquals("true", vars.get(SessionVariable.ENABLE_QUERY_CACHE_INCREMENTAL)); + Assertions.assertEquals("true", vars.get(SessionVariable.QUERY_CACHE_FORCE_REFRESH)); + Assertions.assertEquals("4096", vars.get(SessionVariable.QUERY_CACHE_ENTRY_MAX_BYTES)); + Assertions.assertEquals("64", vars.get(SessionVariable.QUERY_CACHE_ENTRY_MAX_ROWS)); + + SessionVariable master = new SessionVariable(); + Assertions.assertFalse(master.getEnableQueryCache()); + Assertions.assertFalse(master.getEnableQueryCacheIncremental()); + master.setForwardedSessionVariables(vars); + Assertions.assertTrue(master.getEnableQueryCache()); + Assertions.assertTrue(master.getEnableQueryCacheIncremental()); + Assertions.assertTrue(master.isQueryCacheForceRefresh()); + Assertions.assertEquals(4096, master.getQueryCacheEntryMaxBytes()); + Assertions.assertEquals(64, master.getQueryCacheEntryMaxRows()); + } + @Test public void testInsertVisibleTimeoutReturnMode() throws Exception { connectContext.setThreadLocalInfo(); diff --git a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy index a8c1d932069b96..9ee2d876550e3c 100644 --- a/regression-test/suites/query_p0/cache/query_cache_incremental.groovy +++ b/regression-test/suites/query_p0/cache/query_cache_incremental.groovy @@ -260,10 +260,12 @@ suite("query_cache_incremental") { // threshold bounds prior write-backs, not the versions accumulated while // the entry is idle, so the delta of the first stale query after a long // idle stretch can be arbitrarily large. Such a delta must flow through - // the parallel scanner builder like any full scan (split by segment rows) - // and still merge correctly with the cached blocks. enable_parallel_scan - // is on by default; the tiny per-scanner row bound forces the builder to - // really split the delta into many scanners instead of collapsing to one. + // the parallel scanner builder like any full scan and still merge + // correctly with the cached blocks. Both settings below are needed for + // the split to actually happen at regression scale: the per-scanner row + // floor defaults to 2M rows, and BE clamps this variable up to 1024, so + // 1024 is the lowest floor reachable and the delta further down is sized + // several times past it. sql "set enable_parallel_scan=true" sql "set parallel_scan_min_rows_per_scanner=16" sql "DROP TABLE IF EXISTS test_query_cache_incremental_dormant" @@ -297,22 +299,17 @@ suite("query_cache_incremental") { GROUP BY url """ sql "INSERT INTO test_query_cache_incremental_dormant VALUES ('2026-01-01',1,'/a',10)" - // Fill the entry at the small initial version, then leave it dormant - // while 30 loads land on the single-tablet partition: a 30-rowset, - // couple-hundred-row suffix that splits into a dozen or more scanners - // under the 16-row bound. + // Fill the entry at the small initial version, then leave it dormant while + // 30 loads land on the single-tablet partition: a 30-rowset, 6000-row + // suffix. Against the 1024-row floor the builder cuts that into four or + // five scanners, so the delta really goes through the rowset/segment split + // rather than through one serial scanner. checkConsistency(dormantQuerySql) for (int i = 1; i <= 30; i++) { - sql """ - INSERT INTO test_query_cache_incremental_dormant VALUES - ('2026-01-02',${i},'/a',${i}), - ('2026-01-03',${i},'/b',${i}), - ('2026-01-04',${i},'/c',${i}), - ('2026-01-05',${i},'/d',${i}), - ('2026-01-06',${i},'/e',${i}), - ('2026-01-07',${i},'/f',${i}), - ('2026-01-08',${i},'/g',${i}) - """ + def values = (1..200).collect { n -> + "('2026-01-0${(n % 7) + 2}',${i * 1000 + n},'/u${n % 5}',${n})" + }.join(",") + sql "INSERT INTO test_query_cache_incremental_dormant VALUES ${values}" } // The first query after the idle stretch must still take the incremental // path on local storage and agree with the uncached result. The loads From 789c8b4158f2e891c26425828bac114c13256cbb Mon Sep 17 00:00:00 2001 From: Benedict Jin Date: Sat, 18 Jul 2026 00:29:36 +0800 Subject: [PATCH 9/9] [fix](query-cache) Pin the cached side while classifying a merge-on-write delta Address the P1 on the incremental merge decision. The delta capture pins only the delta rowsets, so on a merge-on-write table the rewrite markers classification looks for have no other owner. Those markers live on the rowsets the cached snapshot was built from, and a compaction spanning the cached and delta versions retires such a rowset without relocating an overwritten row's marker: the row has no counterpart in the compaction output, so calc_compaction_output_rowset_delete_bitmap skips it and the marker stays on the retired input. The stale sweep then hands that input to the unused-rowset GC, which drops its delete bitmap. If that lands between the delta capture and the bitmap scan, _delta_rewrites_history sees no rewrite, the cached aggregate is merged with the rows that replaced it, and a wrong entry is stored at the current version and served by later exact hits. The classification now pins the cached side first: under the tablet header lock it captures the rowsets of [0, cached_version] and holds them across _delta_rewrites_history. start_delete_unused_rowset() collects a retired rowset only once nothing else references it, and only a collected rowset reaches remove_rowset_delete_bitmap(), so the reference kept here keeps the evidence too. A quiet capture returns the prefix it walked, so the pinned path is checked to cover the whole [0, cached_version] range; a short prefix (its own history compacted away and swept) falls back to a full recompute rather than classifying against evidence it cannot vouch for. Tests: two cases against a real storage engine. One pins the cached side across a merge-on-write classification and asserts, through a sync point, that the full [0, cached_version] range is held at that moment; the other gives the cached side a hole so the quiet capture returns a short prefix and asserts the coverage check falls back. Both fail on the pre-fix code (the first because the pin is not held, the second because the incremental path is taken instead of the fallback). --- be/src/runtime/query_cache/query_cache.cpp | 41 +++++++++++++++ be/test/exec/pipeline/query_cache_test.cpp | 59 ++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/be/src/runtime/query_cache/query_cache.cpp b/be/src/runtime/query_cache/query_cache.cpp index db0abf2646a64f..d7e042e0301a24 100644 --- a/be/src/runtime/query_cache/query_cache.cpp +++ b/be/src/runtime/query_cache/query_cache.cpp @@ -346,6 +346,47 @@ bool QueryCacheRuntime::_capture_tablet_delta(int64_t tablet_id, int64_t cached_ decision->incremental_fallback_reason = "delta contains delete predicates"; return false; } + // Pin the cached side for as long as the classification below reads the + // delete bitmap. The markers it looks for sit on the rowsets the cached + // snapshot was built from, and the capture above pins only the delta + // side, so nothing else keeps that evidence alive: once a compaction + // spanning both sides retires such a rowset, an overwritten row has no + // counterpart in the output and its marker is therefore not relocated + // (BaseTablet::calc_compaction_output_rowset_delete_bitmap skips a source + // row whose id does not convert), leaving the marker only on the retired + // input, which the stale sweep then hands to the unused-rowset GC. + // Classification would see no rewrite and merge the cached rows with + // their replacements: a wrong entry, stored at the current version. + // Holding the rowsets stops that, in start_delete_unused_rowset(): it + // collects a retired rowset only once nothing else references it, and + // only what it collects gets remove_rowset_delete_bitmap(), which drops + // every version of that rowset's bitmap. The other remover, the key + // ranges that the stale sweep queues onto the pre-window rowsets, is + // held off by the delta capture instead: that queue is released only + // once the swept path's own rowsets are unreferenced. + std::vector cached_side_pin; + if (merge_on_write) { + { + std::shared_lock rdlock(tablet->get_header_lock()); + auto pin_res = tablet->capture_consistent_rowsets_unlocked({0, cached_version}, + {.quiet = true}); + if (pin_res) { + cached_side_pin = std::move(pin_res.value().rowsets); + } + } + // quiet yields the prefix it walked, so cover the whole cached range + // before trusting the classification: an unpinned tail is evidence we + // cannot vouch for. + if (cached_side_pin.empty() || cached_side_pin.front()->start_version() != 0 || + cached_side_pin.back()->end_version() != cached_version) { + decision->incremental_fallback_reason = "cached versions not pinnable"; + return false; + } + // The pin is only worth anything while the classification runs, so + // let a test observe it exactly there rather than infer it. + TEST_SYNC_POINT_CALLBACK("QueryCacheRuntime::_capture_tablet_delta.cached_side_pinned", + &cached_side_pin); + } if (merge_on_write && _delta_rewrites_history(*tablet, *read_source, cached_version, decision->current_version)) { // A load in the delta window marked rows of the cached snapshot as diff --git a/be/test/exec/pipeline/query_cache_test.cpp b/be/test/exec/pipeline/query_cache_test.cpp index ba91148307ff8d..4f79f4e8943146 100644 --- a/be/test/exec/pipeline/query_cache_test.cpp +++ b/be/test/exec/pipeline/query_cache_test.cpp @@ -900,6 +900,65 @@ TEST_F(QueryCacheIncrementalTest, mow_history_rewrite_falls_back) { EXPECT_EQ(decision->incremental_fallback_reason, "delta rewrites history rows"); } +TEST_F(QueryCacheIncrementalTest, mow_cached_side_pinned_across_classification) { + // The rewrite markers classification looks for live on the cached side, + // which the delta capture does not pin. Unpinned, a compaction spanning + // both sides can retire such a rowset without relocating the marker of a + // row it dropped, and the unused-rowset GC can then wipe that marker + // before classification reads it -- a stale entry would silently merge + // with the rows that replaced it. So the pin must cover the whole cached + // range and must still be held while classification runs, which is where + // this sync point observes it. + ASSERT_NE(create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 50}, {51, 100}}), nullptr); + auto* sp = SyncPoint::get_instance(); + int64_t pinned_start = -1; + int64_t pinned_end = -1; + int observed = 0; + sp->set_call_back("QueryCacheRuntime::_capture_tablet_delta.cached_side_pinned", + [&](auto&& args) { + auto* pinned = try_any_cast*>(args[0]); + ++observed; + EXPECT_FALSE(pinned->empty()); + if (!pinned->empty()) { + pinned_start = pinned->front()->start_version(); + pinned_end = pinned->back()->end_version(); + } + }); + sp->enable_processing(); + Defer clear_sp {[&] { + sp->disable_processing(); + sp->clear_all_call_backs(); + }}; + + auto decision = make_stale_decision(); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::INCREMENTAL); + // Guard against a vacuous pass: a renamed or removed sync point would + // leave the pin unobserved while the assertions below still held. + EXPECT_EQ(observed, 1); + // The cached snapshot is everything up to the cached version, and every + // bit of it must be pinned: an unpinned tail is evidence we could lose. + EXPECT_EQ(pinned_start, 0); + EXPECT_EQ(pinned_end, 50); +} + +TEST_F(QueryCacheIncrementalTest, mow_cached_side_not_pinnable_falls_back) { + // Part of the cached snapshot is gone (compacted away and swept), so its + // rewrite markers may already have been collected: classification could no + // longer tell an append from an overwrite. The delta is unaffected and + // still captures, so only the cached-side pin can catch this. The pin asks + // quietly, which yields whatever prefix it walked -- here [0, 40], short + // of the cached version 50 -- so it is the coverage check, not emptiness, + // that must reject it. + ASSERT_NE(create_tablet(TKeysType::UNIQUE_KEYS, true, {{0, 40}, {51, 80}, {81, 100}}), nullptr); + int64_t fallbacks_before = + DorisMetrics::instance()->query_cache_incremental_fallback_total->value(); + auto decision = make_stale_decision(); + EXPECT_EQ(DorisMetrics::instance()->query_cache_incremental_fallback_total->value(), + fallbacks_before + 1); + EXPECT_EQ(decision->mode, QueryCacheInstanceDecision::Mode::MISS); + EXPECT_EQ(decision->incremental_fallback_reason, "cached versions not pinnable"); +} + TEST_F(QueryCacheIncrementalTest, mow_irrelevant_bitmap_entries_ignored) { // Delete-bitmap entries that cannot affect the cached snapshot must not // spoil the incremental path: entries targeting the delta rowsets (a key