Doris-Breakwater commented on issue #66537: URL: https://github.com/apache/doris/issues/66537#issuecomment-5202014104
Breakwater-GitHub-Analysis-Slot: slot_2736850baa2c ### Maintainer triage This is a **high-severity, source-confirmed null dereference in the ROW-binlog rollup create path**. No labels are currently assigned; `bug`, `binlog`, and `rollup/schema-change` would be appropriate triage areas. #### Verified mechanism at `40762c79bd014d66a843cf21ab2a1464e5bac092` 1. [`RollupJobV2.createRollupReplica()`](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/fe/fe-core/src/main/java/org/apache/doris/alter/RollupJobV2.java#L224-L275) copies the table's enabled ROW-binlog configuration into every rollup `CreateReplicaTask`, but passes `null` as the final `rowBinlogMeta` argument. 2. [`CreateReplicaTask.toThrift()`](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/fe/fe-core/src/main/java/org/apache/doris/task/CreateReplicaTask.java#L460-L504) always sets `binlog_config` when it is non-null, while setting `row_binlog_schema` only when ROW binlog is enabled **and** `rowBinlogMeta != null`. The resulting rollup request therefore has `binlog.enable=true`, `binlog.format=ROW`, and no `row_binlog_schema`. 3. On the BE, [`TabletMeta::create()` / the constructor](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/be/src/storage/tablet/tablet_meta.cpp#L111-L131) preserves the enabled ROW-binlog configuration, while the constructor initializes `_row_binlog_schema` only when that optional request field is present ([lines 209-245](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/be/src/storage/tablet/tablet_meta.cpp#L209-L245)). Thus `_row_binlog_schema` remains null. 4. [`Tablet::enable_row_binlog()`](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/be/src/storage/tablet/tablet.h#L519-L522) checks only the config. Consequently [`create_initial_rowset()`](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/be/src/storage/tablet/tablet.cpp#L2211-L2223) enters the ROW-binlog branch and passes the null `row_binlog_tablet_schema()` into the second rowset writer. 5. `BaseBetaRowsetWriter::init()` has only a `DCHECK` for the pointer, which is ineffective in a release build, and [`RowsetMeta::set_tablet_schema()`](https://github.com/apache/doris/blob/40762c79bd014d66a843cf21ab2a1464e5bac092/be/src/storage/rowset/rowset_meta.cpp#L223-L229) immediately evaluates `tablet_schema->to_key()`. That produces the reported `to_key() -> to_schema_pb()` stack. The first access to `_cluster_key_uids` also explains the near-null `0x150` fault address. Therefore, the bad runtime candidate is specifically `RowsetWriterContext::tablet_schema` for the **row-binlog initial writer**, not the ordinary rollup tablet schema. This source defect explains the supplied stack without requiring memory pressure or a stale base-tablet schema. The same request construction and BE dereference were still present on upstream `master` at `4e3c1b84dd5936b8d751e0a95719350e8cd8e980` when checked. #### Scope and remaining uncertainty - `INSERT OVERWRITE` is unlikely to be a prerequisite: none of the request construction above depends on partition-replacement history. The minimal trigger is likely an enabled ROW-binlog table followed by creation of a rollup tablet. This still needs an isolated run to confirm the incident exercised exactly this deterministic path. - Reusing the base tablet's data directory is expected for rollup/schema-change creation and is orthogonal to the null schema. - The shared `CreateReplicaTask` behavior is also used by other non-base-index creation/recovery paths. Those callers should be audited for the same combination of enabled ROW-binlog config plus null `rowBinlogMeta`; their runtime impact has not been reproduced here. #### Recommended next steps 1. Confirm the minimal reproduction on the reported commit by removing `INSERT OVERWRITE` and running only `CREATE TABLE ... binlog.format=ROW` followed by `ADD ROLLUP`. Retain the original sequence as a regression case as well. 2. Fix the FE request invariant after confirming the intended design: because ROW binlog metadata is base-index-only, rollup/non-base tablet requests should not advertise enabled ROW binlog unless they also carry a valid row-binlog schema. 3. Add BE request-boundary validation for the inconsistent combination (`ROW` enabled but `row_binlog_schema` absent) and return a diagnostic `Status` instead of relying on `DCHECK`. This is important for compatibility with an older FE sending the malformed request. 4. Add an FE unit assertion on the generated rollup `TCreateTabletReq`, a BE unit test for the malformed request, and a regression test that verifies the rollup completes and all BEs remain alive. If the minimized run does not reproduce, please attach the serialized/logged `TCreateTabletReq` field presence for `binlog_config` and `row_binlog_schema`, together with the rollup/base tablet and index IDs. Core values for the two `RowsetWriterContext::tablet_schema` instances would then distinguish this path from unrelated corruption. -- 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]
