Doris-Breakwater commented on issue #70427: URL: https://github.com/apache/doris/issues/70427#issuecomment-6760965736
Breakwater-GitHub-Analysis-Slot: slot_abf301325dd4 This merits priority investigation as a potential query correctness defect. The reported same-key result change when disabling short-circuit execution is strong evidence for investigating that path. It does **not yet establish which component returns stale data**. The issue is currently open with no labels; suggested triage areas are BE point queries, row storage, and MoW correctness. I inspected the public **4.1.3** source, resolving to commit `7126cf65d96ebc43fce0906f51e92c1a2ccf24a6`. This is source analysis, not an independently executed reproduction; please provide the actual FE/BE build hashes to confirm the deployed code matches. **What the code establishes** - The FE short-circuit rewrite recognizes a result sink over an optional projection, a key-equality filter, and an OLAP scan. Its patterns do not include a LIMIT node. A retained LIMIT can therefore change eligibility; compare both actual EXPLAIN outputs rather than treating LIMIT as a correctness guarantee. See the [4.1.3 rewrite rule](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/LogicalResultSinkToShortCircuitPointQuery.java#L63-L112). - `enable_sql_cache` does not control the separate **BE storage RowCache**. If enabled, this cache uses only tablet ID plus encoded primary key, without a rowset/version component. A hit skips primary-key lookup and directly deserializes the cached row. On a miss, row-store reading can refill it. See [cache key](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/service/point_query_executor.h#L132-L146), [lookup/materialization](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/service/point_query_executor.cpp#L452-L532), and [refill](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/storage/tablet/base_tablet.cpp#L500-L529). Crucially, [4.1.3 defaults `disable_storage_row_cache` to `true`](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/common/config.cpp#L415-L416). RowCache is relevant only if the runtime configuration enables it. - There is a concrete conditional invalidation gap worth checking against the omitted table properties: reads can cache **partial row storage** (`row_store_columns`), but both writers invalidate only when `has_row_store_for_all_columns()` is true and the write is `TYPE_DIRECT`. The [schema predicate](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/storage/tablet/tablet_schema.h#L491-L494) excludes partial row storage; see [SegmentWriter](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/storage/segment/segment_writer.cpp#L373-L381) and [VerticalSegmentWriter](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/storage/segment/vertical_segment_writer.cpp#L359-L367). Thus a previously cached old row can survive a sequential UPDATE under those conditions. **The issue does not establish that those conditions apply to this table.** - For full row storage with RowCache enabled, investigate publication timing and replica write paths separately. Writer-side invalidation happens before the new rowset becomes visible, allowing a potential old-read refill window. Also, [segment-file receivers](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/load/channel/load_stream_writer.cpp#L100-L140) append file bytes rather than running those per-key writer hooks; the inspected commit/publish path contains no compensating RowCache erase. Whether either mechanism was exercised here remains unverified. - With RowCache bypassed, the lookup does not simply ignore MoW visibility: it searches [rowsets in descending end-version order](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/storage/tablet/base_tablet.cpp#L484-L497) and [checks the delete bitmap](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/be/src/storage/tablet/base_tablet.cpp#L596-L624). Different historical results alone cannot prove a delete-bitmap defect. The FE also [shuffles candidate backends](https://github.com/apache/doris/blob/7126cf65d96ebc43fce0906f51e92c1a2ccf24a6/fe/fe-core/src/main/java/org/apache/doris/qe/PointQueryExecutor.java#L130-L140), so per-BE correlation matters with three replicas. [Issue #34056](https://github.com/apache/doris/issues/34056) concerns NULL/default-value materialization. [PR #65051](https://github.com/apache/doris/pull/65051) addresses nested-column pruning descriptors and complex-column materialization crashes. Neither establishes a fix for this UPDATE/stale-version report, and there is insufficient evidence to recommend a particular fixed release. **Information needed to distinguish these mechanisms** 1. Complete, sanitized `SHOW CREATE TABLE`, especially `row_store_columns`, sequence-column properties, and schema-change history; actual build hashes for every FE/BE; integrated-storage versus storage/compute-separated deployment; runtime `disable_storage_row_cache` on every involved BE. 2. A runnable SQL transcript without omitted columns/properties: INSERT, any reads before UPDATE, UPDATE, its complete response/transaction status, and repeated SELECT results. Confirm the update became **VISIBLE**, whether the same session performed both operations, whether prepared statements were used, and whether concurrent writers/readers were present. 3. Exact SQL and EXPLAIN for no LIMIT, the actual LIMIT value, and short-circuit disabled. Include timestamps/query IDs and the differing row values. 4. For an affected key, correlate the returned value with selected BE/tablet, row-cache hit status, replica versions, and FE/BE publish logs. An aggregate cache-hit counter alone cannot establish that the stale request hit cache. **Next steps and containment** Keep the reporter-validated `SET enable_short_circuit_query = false` workaround in affected sessions. Do not rely on automatically appended LIMIT without verifying the resulting plan. In an isolated reproduction, if RowCache is enabled, repeat with **`disable_storage_row_cache=true` on every candidate BE**, read back the settings, and let earlier in-flight requests finish. This is a BE configuration, not a SQL session variable. Source inspection shows it bypasses both cache lookup and refill while retaining eligible short-circuit execution; its effectiveness for this incident still needs verification. Toggling it does not clear existing entries, so immediately re-enabling cache is not a cache-clearing procedure. For maintainers, first compare full versus partial row storage using `INSERT A → point-read A to warm cache → UPDATE B → repeated point-read`, initially with one replica and no concurrency, then with three replicas and reads overlapping publication. If stale reads persist with RowCache disabled everywhere, trace the selected replica's rowsets, primary-key location, delete bitmap, and row-store payload against the normal-scan result. Any eventual fix needs regression coverage for cache warm-up, sequential UPDATE, partial/full row storage, and publication/concurrent refill across replicas. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
