Doris-Breakwater commented on issue #66571: URL: https://github.com/apache/doris/issues/66571#issuecomment-5213475080
Breakwater-GitHub-Analysis-Slot: slot_449674ca69e5 ### Triage verdict Confirmed as an actionable FE/binlog correctness bug by static code-path review. The report is internally consistent and the proposed configuration snapshot is directionally correct. No additional logs are required to establish the immediate root cause. At intake this issue is open with no labels, assignee, milestone, or prior comments. ### Verified facts - On `3.1.4-rc02` (`7f5ba43de6c7f148e1dacdb7cb838394d2ec1537`), `Alter.processReplaceTable()` calls `replaceTableInternal()` before constructing and logging `ReplaceTableOperationLog` ([source](https://github.com/apache/doris/blob/7f5ba43de6c7f148e1dacdb7cb838394d2ec1537/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java#L680-L684)). - For `swap=false`, `replaceTableInternal()` unregisters both names, registers only the replacement under the original name, and does not re-register `origTable` ([source](https://github.com/apache/doris/blob/7f5ba43de6c7f148e1dacdb7cb838394d2ec1537/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java#L736-L748)). - `BinlogManager.addReplaceTable()` attaches only `origTblId`, then the generic `addBinlog()` asks `BinlogConfigCache.isEnableTable(dbId, origTblId)` ([source](https://github.com/apache/doris/blob/7f5ba43de6c7f148e1dacdb7cb838394d2ec1537/fe/fe-core/src/main/java/org/apache/doris/binlog/BinlogManager.java#L427-L440)). On a miss, the cache resolves the ID from the live catalog; after `swap=false` that lookup returns no table and therefore no enabled table config ([source](https://github.com/apache/doris/blob/7f5ba43de6c7f148e1dacdb7cb838394d2ec1537/fe/fe-core/src/main/java/org/apache/doris/binlog/BinlogConfigCache.java#L89-L130)). - The follower path has the same ordering: `EditLog.loadJournal()` first calls `replayReplaceTable()` and only afterward `addReplaceTable()` ([source](https://github.com/apache/doris/blob/7f5ba43de6c7f148e1dacdb7cb838394d2ec1537/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java#L1051-L1055)). - PR #48919 changes the single-table assignment to OR with the DB-level result. That prevents a missing table lookup from overriding `dbBinlogEnable=true`, but with DB binlog disabled the result still depends on resolving the removed original table. It does not fix this table-only case. - The same catalog-before-binlog ordering and cache-dependent predicate are still present on current upstream master (`c1bab9e8cf80fc5e0c52914e9d4aa69717ea2e7a`). This explains the missing `REPLACE_TABLE` event without assuming any CCR-side fault. A warm cache can mask the defect, so behavior is incorrectly cache-state-dependent. ### Fix requirements Capturing a copy of the original table's `BinlogConfig` while the table locks are held and before unregistering it, then persisting it in `ReplaceTableOperationLog`, is the appropriate source of truth. The implementation should also cover all consumers of that historical configuration: 1. Live master emission and follower edit-log replay must decide from the persisted snapshot (plus DB-level enablement), not from a post-mutation catalog lookup. 2. The resolved configuration must reach the `DBBinlog`/`TableBinlog` path as well, and remain available for table-level TTL/max-bytes/max-history GC after the original table leaves the catalog. Changing only the top-level `anyEnable` predicate is not sufficient as a robust fix. 3. Checkpoint/image recovery needs explicit coverage. `BinlogManager.write()` stores table dummies before common binlogs, while `read()` currently filters a table dummy through `isEnableTable()` before it encounters the serialized `REPLACE_TABLE` record. Once the original table is absent, a restart can therefore fail to reconstruct that original table's `TableBinlog` even if the live FE emitted the event. Recovery should restore/use the persisted snapshot before filtering or reconstructing that table binlog. 4. Older `ReplaceTableOperationLog` JSON has no snapshot. Deserialization and replay must remain null-safe; retain the legacy behavior where a reliable snapshot is unavailable rather than failing replay. ### Missing information Nothing is blocking bug acceptance. The exact FE build/commit identifier would only be useful for deciding the precise 3.1.x backport target and whether that build already carries a backport of #48919. ### Recommended next steps - Implement the persisted pre-mutation snapshot and the live/replay/image-recovery handling above. - Add FE tests for `db=false`, original table binlog enabled, `swap=false`, and a deliberately cold cache; assert that `getBinlog(origTblId, ...)` returns `REPLACE_TABLE` on both the writer and replay paths. - Add a BinlogManager serialize/read test with the original table already absent, plus warm-cache/cold-cache equivalence and legacy-log-without-snapshot compatibility tests. - Keep the separate `swap=true`/table-ID attachment concern tracked independently; this issue's verified failure is the `swap=false`, table-level-enable decision. -- 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]
