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

   Confirmed: this is a BE parsing/identity defect triggered by the uppercase 
IDs generated for cloud initial rowsets. Source inspection confirms the 
mismatch in the local `4.1.4` tag and in the cited master commit 
`351d9388d5b945e656f5f91df17d81b5bd4205b3`. The issue currently has no labels 
or assignee; the relevant areas are cloud metadata, FE initialization, and BE 
ID utilities.
   
   The verified path is:
   
   1. FE creates a visible, empty `[0-1]` rowset with zero rows and zero 
segments and formats its UUID halves with uppercase hex. See [4.1.4 
initialization](https://github.com/apache/doris/blob/4.1.4/fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java#L410-L436)
 and [the cited master 
implementation](https://github.com/apache/doris/blob/351d9388d5b945e656f5f91df17d81b5bd4205b3/fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java#L690-L693).
   2. BE's [RowsetMeta 
initialization](https://github.com/apache/doris/blob/4.1.4/be/src/storage/rowset/rowset_meta.cpp#L267-L273)
 passes `rowset_id_v2` to `RowsetId::init`. In `4.1.4`, that parser lives in 
[olap_common.h](https://github.com/apache/doris/blob/4.1.4/be/src/storage/olap_common.h#L492-L516);
 in the cited master commit it lives in 
[rowset_id.cpp](https://github.com/apache/doris/blob/351d9388d5b945e656f5f91df17d81b5bd4205b3/be/src/storage/rowset_id.cpp#L57-L63).
   3. 
[from_hex](https://github.com/apache/doris/blob/4.1.4/be/src/util/uid_util.h#L48-L62)
 treats uppercase letters as negative integers, which contaminate the 
accumulated value through bitwise OR. The [master 
parser](https://github.com/apache/doris/blob/351d9388d5b945e656f5f91df17d81b5bd4205b3/be/src/util/uid_util.h#L48-L62)
 has the same behavior. The string-based `UniqueId` constructor uses this 
utility too; other callers' exposure to uppercase input needs separate 
confirmation.
   
   I compiled and ran an isolated C++20 reproducer using the verbatim `4.1.4` 
hex templates and `RowsetId` struct, with unrelated error/logging dependencies 
stubbed. The supplied uppercase example serializes as:
   
   ```text
   0200000000000000ffffef0123456789ffffef0123456789
   ```
   
   The lowercase control preserves the intended numeric value. These two 
distinct inputs, whose UUID portions satisfy the v4 version/variant bit layout, 
also compare equal after parsing:
   
   ```text
   02000000000000001111111111114A018111111111111B02
   02000000000000002222222222224A019222222222223B02
   => 0200000000000000ffffffffffffea01fffffffffffffb02
   ```
   
   This establishes lossy parsing and possible collisions. It was an isolated 
reproducer, not a full Doris unit-test or live-cluster run.
   
   There is an additional compatibility detail: fixing uppercase parsing alone 
restores numeric identity, but `to_string()` still emits lowercase. Existing 
uppercase metadata will therefore still fail the raw string equality check in 
[sync_tablet_rowsets](https://github.com/apache/doris/blob/4.1.4/be/src/cloud/cloud_meta_mgr.cpp#L950-L969).
 That comparison should use equivalent parsed IDs or consistent 
canonicalization. Ordinary `RowsetMeta` initialization retains the original 
protobuf string, so a protobuf-backed JSON view can differ from logs that print 
the parsed ID; parsing does not itself demonstrate that persisted metadata was 
rewritten.
   
   Operational impact remains bounded by the evidence. These initial rowsets 
contain no data segments. Also, 
[CloudTablet::add_rowsets](https://github.com/apache/doris/blob/4.1.4/be/src/cloud/cloud_tablet.cpp#L437-L445)
 subsequently filters duplicates by numeric ID, and unused-rowset tracking is 
per tablet. Thus the collision and string mismatch do not, by themselves, 
establish data loss or a reference-count leak. A suspicious hex pattern alone 
also does not identify a version-hole rowset: [hole 
creation](https://github.com/apache/doris/blob/4.1.4/be/src/cloud/cloud_meta_mgr.cpp#L2534-L2593)
 constructs a deterministic ID from tablet/version and explicitly marks the 
rowset as a hole.
   
   Recommended next steps for the proposed PR:
   
   - Accept both hex cases in BE so existing uppercase metadata parses 
correctly; emit lowercase from FE for newly created initial rowsets. An FE-only 
change leaves existing metadata affected.
   - Cover the sync comparison against existing uppercase metadata, rather than 
assuming a parser change fixes byte-for-byte string equality.
   - Add tests for uppercase/lowercase/mixed-case numeric equivalence, the 
collision pair above, lowercase serialization, and the string-based `UniqueId` 
constructor. Include a cloud sync test using an existing uppercase 
initial-rowset ID and preserve legacy rowset-ID behavior. Define and test 
invalid-input handling separately instead of silently mapping invalid 
characters.
   
   No further information is needed to establish the parser defect. To assess 
observed cluster impact, please provide the exact FE/BE build hashes, one 
original meta-service ID paired with its BE-rendered ID for the same tablet and 
`[0-1]` version, and the precise HTTP endpoint or log location used. If 
reference-retention symptoms are also observed, include the corresponding 
timestamped warnings and sync/compaction context; that causal link is not yet 
verified.
   
   Breakwater-GitHub-Analysis-Slot: slot_f180809562c9
   


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