zclllyybb commented on issue #63927:
URL: https://github.com/apache/doris/issues/63927#issuecomment-4589095295

   Breakwater-GitHub-Analysis-Slot: slot_da5512a13b11
   
   Initial analysis:
   
   This looks like a real BE UT build/linkage bug, not a test flake. I 
rechecked the live issue; at read time it had no labels and no existing 
comments. I also inspected Doris at commit `4616202fc4`.
   
   Code evidence:
   
   - `SegmentWriter` declares `_is_mow()` and `_is_mow_with_cluster_key()` in 
`be/src/storage/segment/segment_writer.h`, while their definitions are `inline` 
only in `be/src/storage/segment/segment_writer.cpp`.
   - `TestSegmentWriter::append_row()` in 
`be/test/storage/segment/test_segment_writer.h` calls both helpers from test 
code, so optimized test objects can need linkable definitions outside the 
`.cpp` that contains the inline bodies.
   - `VerticalSegmentWriter` has the same header declaration plus `.cpp`-local 
inline definition pattern for the same two helpers.
   
   Judgment:
   
   The root cause described in the issue is consistent with the code. A 
`.cpp`-local `inline` definition is fragile here: optimized builds may not emit 
an out-of-line symbol, while other translation units only see the declaration 
and can still generate normal calls. That matches the reported TSAN/Release 
undefined references and also explains why an `-O0` ASAN build can mask the 
problem.
   
   The proposed fix direction is reasonable. Either remove `inline` and keep 
ordinary out-of-line `.cpp` definitions, or move the inline definitions into 
the headers so every translation unit that may call them sees the bodies. Given 
the test/header call path, header-inline definitions are a minimal fix.
   
   Current status and next steps:
   
   - PR #63928 is already open and implements the header-inline direction.
   - Before merge, please require the reported command or an equivalent 
optimized BE UT link step, for example `BUILD_TYPE_UT=TSAN bash run-be-ut.sh 
--run --filter=DeleteBitmapCalculatorTest.*`. A normal ASAN/`-O0` build is not 
sufficient validation for this failure mode.
   - One review item for PR #63928: if the new coverage adds direct test 
subclasses that call these helpers, please confirm the helpers are accessible 
under C++ access rules. The existing failing `SegmentWriter` path goes through 
`TestSegmentWriter`, which is already a friend; additional direct subclass 
tests may need explicit access handling.
   
   Missing information:
   
   - The full linker command/log would make the exact unresolved object-file 
references explicit.
   - A passing TSAN or Release BE UT build result after PR #63928 is the main 
confirmation needed.
   


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