Doris-Breakwater commented on issue #66368: URL: https://github.com/apache/doris/issues/66368#issuecomment-5161876944
Initial assessment: **confirmed BE copy-on-write race on the reported commit, with high confidence in the root cause**. The failure is not explained by a supported resource limit. The same unsafe ownership pattern is still present on current `master` (`dc4fc151b997a6d1909e01ebdc6bcc60eed5b8a2`). This should be treated as a high-priority binlog/storage stability defect. The issue currently has no labels, assignee, milestone, or linked fix. ### Verified code path 1. When ROW binlog uses a `GroupRowsetWriter`, [`FlushToken::submit()` creates two independently scheduled tasks](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/load/memtable/memtable_flush_executor.cpp#L173-L210), one for the normal data writer and one for the row-binlog writer. `_memtable2block()` constructs the block once and [returns the same `SharedMemtable::block` to both tasks](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/load/memtable/memtable_flush_executor.cpp#L293-L315); each task then passes it to its child writer ([call site](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/load/memtable/memtable_flush_executor.cpp#L391-L405)). 2. `SegmentFlusher` makes only a shallow `Block` copy. With binlog disabled in the data child context it selects `VerticalSegmentWriter`; with binlog enabled in the binlog child context it selects `RowBinlogSegmentWriter` ([selection](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/storage/rowset/segment_creator.cpp#L68-L86)). This accounts for both reported stacks reaching the same converter from the two concurrent sub-tasks. 3. `OlapBlockDataConvertor::set_source_column()` retains another `ColumnPtr` to the selected top-level column ([code](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/storage/iterator/olap_data_convertor.cpp#L313-L325)). For a nullable simple column, however, `OlapColumnDataConvertorSimple::convert_to_olap()` obtains the nested column by reference and calls: ```cpp std::move(nullable_column->get_nested_column()).mutate() ``` ([code](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/storage/iterator/olap_data_convertor.h#L303-L328)). This does not transfer or replace the `ColumnNullable` owner slot. Copies of the top-level `ColumnPtr` therefore do not raise the nested column's refcount; both flush threads can observe that nested node as uniquely owned and try to acquire a mutable pointer to it. 4. The exact exception follows from the two separate refcount observations in COW: `shallow_mutate()` first checks `use_count()`, then the unique path calls `assert_mutable()`, which checks it again ([code](https://github.com/apache/doris/blob/1590e7e3228f8f0cafcaa89e841fece09c2475be/be/src/core/cow.h#L308-L327)). One thread can pass the first check while the count is 1; another thread then returns a mutable pointer and increments the count; the first thread reaches `assert_mutable()` with a count greater than 1 and emits the reported error. A different interleaving can let both threads pass the assertion and create simultaneous mutable aliases to the nested column. That is a genuine ownership violation, not merely an over-strict diagnostic. It is especially relevant for nullable `FLOAT`/`DOUBLE`, because this converter normalizes NaNs in place in the same function; other simple types still take an unnecessary mutable alias. This also explains why round 1 may succeed and round 2 fail: the trigger is scheduling-dependent, and a wide schema plus many memtable flushes creates many opportunities for the race. The present evidence does **not** show that the business-key prefix, historical-value lookup, 128 buckets, inverted indexes, memory pressure, or input bytes are independently causal. There is no supplied evidence of persisted data corruption, but the possible shared mutable alias means the problem should not be classified as only a load-cancellation diagnostic. ### Information still useful The code and stacks are sufficient to confirm the ownership defect. The following artifacts would identify the exact runtime column and make the regression portable: - The complete demangled stack for each occurrence, preserving the actual `OlapColumnDataConvertorSimple<T>` specialization, plus the failing column name/id, type, and nullability. - The complete `SHOW CREATE TABLE`, especially all nullable simple columns and any `FLOAT`/`DOUBLE` columns. - Correlated BE log excerpts for the data and row-binlog sub-tasks with timestamp, load ID, tablet ID, rowset/segment ID, and thread ID. - The referenced `ivm_performance_scale_matrix_test.groovy`, its data generator, and any required source-file manifest. That path is not present in the public repository at the reported commit or on current master, so maintainers cannot currently run the stated case. A Doris query profile is not required to establish this COW race. ### Recommended next steps 1. Fix the converter ownership boundary. Do not call rvalue `mutate()` on a borrowed nested-column reference. Either detach through an owned `ColumnPtr`/the parent owner slot before accessing a writable nested node, or keep the normal conversion path const and create a private writable temporary only for types that actually require normalization. A whole-block deep copy or serializing the two flush tasks would mask the race at substantially broader cost and should not substitute for fixing the converter contract. 2. Add a deterministic BE test using the real grouped memtable flush path, a two-thread pool, and nullable fixed-width columns. Synchronize the data and row-binlog writers at conversion so both touch the same nested node. Include nullable `FLOAT`/`DOUBLE` with NaN, verify the source block remains unchanged, and verify that both the data and binlog segments finish correctly. The existing group-flush test uses non-nullable columns and mock writers, so it does not exercise this converter race. 3. Exercise both normal segment-writer variants after the fix: vertical enabled and disabled, ROW binlog with historical values enabled and disabled, and repeated concurrent flushes under TSAN if available. 4. Audit other calls that apply rvalue `mutate()` to a reference returned from a composite column. The safe API must either consume an owning pointer or replace the composite's owner slot. 5. Add the repository's existing `binlog`, `area/storage`, `area/load`, `kind/stability`, and `kind/need-regression-test` labels as appropriate, then assign a BE storage/binlog owner. Backport evaluation should be based on supported branches containing both the grouped ROW-binlog flush path and the enforced COW contract; no internal branch/tag naming should be inferred from this development build. Breakwater-GitHub-Analysis-Slot: slot_ce2778b8bd08 -- 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]
