Doris-Breakwater commented on issue #70258: URL: https://github.com/apache/doris/issues/70258#issuecomment-6652665948
Breakwater-GitHub-Analysis-Slot: slot_5fbbda800e71 **Assessment:** The missing-status defect is confirmed by source inspection in release tag `4.1.4` (`ad35a140c7fd0b842f18c23300bac581f7d04326`) and master at `bff68004fda41c0649efc9de4c5d14df29069440`. This merits priority as a load-correctness issue. The reported failover/load outcome remains a scenario to reproduce, rather than an observed result of this investigation. The issue currently has no labels; suggested areas are FE RPC handling, loading, and automatic partitioning. **Verified mechanism:** - Both non-master branches construct `NOT_MASTER` and return without attaching it to the result. Other error branches in the same handlers do attach their status. See the master [createPartition handler](https://github.com/apache/doris/blob/bff68004fda41c0649efc9de4c5d14df29069440/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java#L4725-L4744) and [replacePartition handler](https://github.com/apache/doris/blob/bff68004fda41c0649efc9de4c5d14df29069440/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java#L5047-L5068). - Both result structs declare `status` optional. The BE consumers read `result.status.status_code` without checking `result.__isset.status`. Consequently, the absent field is interpreted through the C++ object's default status, rather than rejected as a malformed response. `OK` is enum value zero. See the [result definitions](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/gensrc/thrift/FrontendService.thrift#L1455-L1486), [status definition](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/gensrc/thrift/Status.thrift#L36-L44), and [BE consumers](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/exec/sink/vrow_distribution.cpp#L97-L259). - For creation, an empty partition list adds no routing entries. Both tablet-writer implementations call `automatic_create_partition()` before replaying the buffered block. With `_deal_batched` set, replay uses the ordinary distribution path, without a missing-row collector. Rows still lacking a partition increment the filtered-row counter and record `no partition for this tuple`. See [writer replay](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/exec/sink/writer/vtablet_writer.cpp#L1939-L1959), [distribution selection](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/exec/sink/vrow_distribution.cpp#L578-L592), and [tablet lookup](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/exec/sink/vtablet_finder.cpp#L37-L68). There are two qualifications to the impact description: 1. A positive `max_filter_ratio` alone does not guarantee success: the actual filtered fraction must be within the configured threshold. If every selected row is filtered, ordinary Stream Load requires `max_filter_ratio=1`. INSERT with `enable_insert_strict=false` still checks `insert_max_filter_ratio`. See the [Stream Load check](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/load/stream_load/stream_load_executor.cpp#L103-L120) and [INSERT check](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/AbstractInsertExecutor.java#L240-L254). Accepted filtering is visible in load counters/error records, although the real FE rejection is lost. 2. `replacePartition` should not be assumed to produce the same filtering outcome. Its BE success path calls `replace_partitions(nonempty_old_ids, empty_new_partitions)`. That function has a size-equality `DCHECK`; with that check disabled, the replacement loop is skipped, old entries are erased from `_partitions`, but the lookup map is not updated. This leaves inconsistent local routing state and can leave lookups pointing to original partitions. That is a separate correctness risk to test; it does **not** establish that an overwrite commits to original tablets or loses data end to end. See [replacement implementation](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/tablet_info.cpp#L840-L932) and [map-based lookup](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/tablet_info.h#L216-L234). **Trigger and remaining evidence:** In 4.1.4 these RPCs use the heartbeat-learned master address. Current master also allows a sink-provided master address, falling back to heartbeat state. A stale address is therefore plausible, but the old FE must still answer as a non-master; an unreachable stopped FE instead takes the transport-error path. These callers use a captured address, and the transport helper does not retry an application-level `NOT_MASTER` response. Returning the missing status would propagate failure; automatic redirection would require separate work. See [current address selection](https://github.com/apache/doris/blob/bff68004fda41c0649efc9de4c5d14df29069440/be/src/exec/sink/vrow_distribution.cpp#L126-L149) and [transport helper](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/util/thrift_rpc_helper.cpp#L63-L156). The code evidence is sufficient to proceed with the FE correction. To establish the operational impact, please attach a minimal table DDL and input, exact FE/BE build identifiers, load command/session filter settings, complete load response and error log, and correlated FE role-transition/BE master-address logs with timestamps and load/query/transaction IDs. For overwrite, also capture partition/tablet IDs and table contents before and after. No runtime reproduction or Doris test suite was executed in this investigation. **Recommended next steps:** - Add `result.setStatus(errorStatus)` before both non-master returns, with FE tests asserting `isSetStatus()`, `NOT_MASTER`, and the error message; include a Thrift round-trip assertion for the wire behavior. - Add BE tests using the existing `VRowDistribution.automatic_create_partition.inject_result` and `VRowDistribution.replace_overwriting_partition.inject_result` hooks. An explicit `NOT_MASTER` must stop processing before buffered replay or routing mutation, irrespective of filter tolerance. - Consider BE rejection of an absent status in both consumers, with separate empty-response tests. Keep the Thrift fields optional for compatibility; validate their presence at the consumption boundary. - Reproduce the stale-address window deterministically by directing the RPC to a reachable follower, then test a controlled failover. Cover mixed existing/new partition rows, strict and permissive filter settings, and auto-detect overwrite in builds with and without `DCHECK`. Evaluate backports for affected maintained branches after regression coverage is in place. -- 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]
