zclllyybb commented on issue #65897: URL: https://github.com/apache/doris/issues/65897#issuecomment-5043671321
Breakwater-GitHub-Analysis-Slot: slot_2feae5dd26c0 This content is generated by AI for reference only. Initial code-path check: This should be treated as a real ROW-binlog retention gap unless maintainers decide that ROW binlog intentionally does not support TTL. The reported behavior is consistent with the current implementation: `binlog.ttl_seconds` is accepted together with `binlog.format=ROW`, but I do not find an effective ROW-binlog TTL GC path. Evidence from the reported commit `a4dae0b64e9e7c6db5983b566656442f1ae2bc5d`: - FE `BinlogGcer` calls `BinlogManager.gc()` and sends `BinlogGcTask` to BE based on DB/Table binlog tombstones. FE `BinlogConfig.isEnableForCCR()` explicitly excludes `ROW`, while `isEnableForStreaming()` is the ROW path, so the existing FE tombstone GC path does not cover ROW binlog. - BE `StorageEngine::gc_binlogs()` -> `Tablet::gc_binlogs(version)` deletes ordinary `_binlog` metadata/data/files by FE-provided version. It does not traverse `tablet_meta()->all_row_binlog_rs_metas()` and does not delete tablet `_row_binlog` rowsets. - ROW binlog files are tracked as rowsets in `TabletMeta::_row_binlog_rs_metas` and are read with `capture_row_binlog=true`. The observed `_row_binlog` files remaining is therefore expected with current code unless row-binlog compaction removes/replaces those rowsets. The row-binlog compaction policy is score/size/count/time-threshold driven; I did not find a TTL cutoff using `binlog.ttl_seconds`. `Tablet::binlog_ttl_ms()` exists, but I found no caller. - `IS_STALE` is FE stream metadata (`BaseTableStream.stale`) reported by `TableStreamManager`. I found setters and readers for this flag, but no path that marks a stream stale when a required ROW-binlog range expires or is missing. If a row-binlog version path is absent, BE rowset capture returns a scan error, but that does not update the stream stale flag. I also checked the same symbols on the upstream master head I fetched (`a9816faa6652362b4fc8fbbf057d62652e016624`) and did not find a different ROW-binlog TTL/stale path there. Conclusion: current code does not make ROW binlog entries eligible for physical GC based on `binlog.ttl_seconds`, and it also does not convert ROW-binlog retention loss into an observable stream stale/binlog-broken state. This is best classified as a bug/missing implementation. If the intended contract is instead that TTL only applies to CCR/statement binlog, property validation and documentation should reject or clearly document `binlog.ttl_seconds` for ROW to avoid silent unlimited retention. Suggested next steps: 1. Define the ROW-binlog retention contract. TTL does not need to mean deletion exactly at the boundary, but it should create a reachable eligibility cutoff if the property is accepted. 2. Add BE-side ROW-binlog GC over `_row_binlog_rs_metas` / `_row_binlog` files, coordinated with row-binlog compaction metadata and binlog delete-vector cleanup. 3. Add FE/Table Stream handling for retention loss: a stream or IVM whose required unconsumed range falls below the retained ROW-binlog low watermark should become observable as stale/binlog-broken, or fail with a clear diagnostic and documented recovery/fallback path. 4. Add regression coverage with a very small TTL and waits across FE GC plus BE compaction/GC intervals, verifying both physical cleanup and stream state/diagnostic behavior. No critical information is missing to confirm the code gap. Useful additions for the eventual fix PR would be FE/BE logs around the wait window, `information_schema.table_streams` output before/after the wait, and BE tablet metadata showing `_row_binlog_rs_metas` before/after GC. -- 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]
