Doris-Breakwater commented on issue #70403:
URL: https://github.com/apache/doris/issues/70403#issuecomment-6736076615

   **Initial assessment:** this warrants BE storage/scan triage. The footer 
race in #67719 is a relevant candidate, but the present report does not 
establish that it caused either crash or that one fix covers both. The issue 
currently has no labels, and its body describes two execution contexts without 
providing the actual stacks or reproduction steps.
   
   **Fix status and version coverage — checked on November 9, 2026**
   
   The linked fix is [PR #67720](https://github.com/apache/doris/pull/67720), 
which is **open and unmerged**, targeting `master`. Its `dev/4.1.x` label does 
not establish a merged backport or a shipping release. The current proposal 
removes `_footer_pb` and routes footer reads through `StoragePageCache`; it 
also stops counting footer memory in `Segment::get_metadata_size()`. This 
differs from the mutex proposal originally described in #67719. No specific 
fixed 4.1.x release can be confirmed from that PR's current status.
   
   I checked the local source against the upstream `4.1.5-release` tag, which 
resolves to commit `917b20be83878e98f94e3ebf7efccc80346e3e50`. It **still 
contains the plain weak pointer and unsynchronized accesses**; the proposed 
removal is absent. This identifies the upstream source state, not necessarily 
your deployed binary: please supply its exact build commit and any downstream 
patches. The checked current `branch-4.1` commit, 
`3f4002e778bd4bdac135b349f349e5ed4d7a5093`, also retains the pattern. [4.1.5 
declaration](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/segment/segment.h#L305),
 [footer access 
implementation](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/segment/segment.cpp#L1260).
   
   **What the code supports, and what remains unproven**
   
   - Query scans and compaction both reach rowset/segment readers. 
`LazyInitSegmentIterator` obtains a segment through `SegmentLoader` and invokes 
`Segment::new_iterator()`. The loader can return a shared cached `Segment`, 
including on a lookup hit when the caller does not request cache insertion. 
Thus these workloads can use common storage objects; different planner/context 
flags do not imply separate storage implementations. 
[Iterator](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/segment/lazy_init_segment_iterator.cpp#L35),
 
[loader](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/segment/segment_loader.cpp#L55).
   - `_get_segment_footer()` reads `_footer_pb` and reassigns it after footer 
reload; `get_metadata_size()` also reads it. Concurrent assignment/read of the 
**same** non-atomic weak pointer requires synchronization. `ColumnReaderCache` 
locks its own footer callbacks, but that mutex does not cover every access: 
`traverse_column_meta_pbs()` calls the footer method directly. This is a 
concrete synchronization concern, **not proof of the execution interleaving in 
these incidents**. [Callback 
locking](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/segment/column_reader_cache.cpp#L107),
 [direct 
traversal](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/segment/segment.cpp#L956).
   - `nereids:1` versus `nereids:0 / Query id 0-0` describes thread context, 
not a root-cause signature. Those fields alone neither establish a Nereids 
defect nor prove that the second thread was performing compaction; the stack 
must identify its task. [Crash-header 
fields](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/common/signal_handler.h#L53).
   - The reported disappearance of compaction crashes with 
`disable_auto_compaction=true` is useful isolation evidence. The local-storage 
scheduler checks this setting before generating automatic compactions, so it 
removes relevant work and changes timing/load; it does not repair footer 
synchronization. A lighter cluster remaining healthy is compatible with a 
timing-dependent failure but does not establish one. The rolling upgrade is 
also a temporal association until the old/new builds and crash timeline are 
known.
   - Do not assume the daemon call chain described in #67719 applies here: in 
this 4.1.5 source, `MetadataAdder<Segment>::get_all_segments_estimate_size()` 
reads an aggregate counter rather than invoking individual segments' 
`get_metadata_size()`. Actual stacks and build identity are essential. [Counter 
implementation](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/storage/metadata_adder.h#L137).
   
   **Information needed to determine coverage**
   
   1. One complete, symbolized crash stack for **each** reported family, 
including the signal/fault address, all frames, thread name, timestamp, 
tablet/query IDs, and BE build commit. Preserve the corresponding core and 
matching binary/debug symbols; an all-thread backtrace is particularly useful.
   2. `be.out`, `be.INFO`, and warning/error logs around both crashes, 
including compaction task/tablet/rowset information and any corruption/checksum 
messages. Please identify whether the query-scan crashes also stopped when 
automatic compaction was disabled, and give comparable observation 
duration/load before and after.
   3. Exact pre-upgrade and post-upgrade FE/BE builds, upgrade order, whether 
crashes continued after all nodes finished upgrading, deployment mode, and 
relevant configuration changes.
   4. Affected table DDL, column types (including VARIANT/nested columns), 
indexes, schema-change history, representative query and ingestion workload, 
concurrency, compaction type, cache settings, and reproducible steps or a 
sanitized reproducer. Explain the workload/configuration differences from the 
healthy cluster.
   
   **Suggested next step:** keep both crash families tracked separately until 
their stacks can be matched. Compare them with #67719 and [the sanitizer 
evidence reported on 
#67720](https://github.com/apache/doris/pull/67720#issuecomment-6499912164); 
that evidence supports the race mechanism but is not a reproduction of this 
issue. If the stacks/build support the match, coordinate a reviewed 4.1 
backport/canary of the accepted fix and validate both families under comparable 
scan, compaction, and cache pressure. Confirm the merge, backport, and release 
contents before naming a fixed version. If automatic compaction remains 
disabled as temporary containment, monitor rowset/version accumulation and 
compaction backlog; the setting should not be treated as a permanent resolution.
   
   Breakwater-GitHub-Analysis-Slot: slot_9176e506c167
   


-- 
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]

Reply via email to