luwei16 commented on PR #68138:
URL: https://github.com/apache/doris/pull/68138#issuecomment-5748539155
Local review result: **PASS**.
Reviewed the exact three-dot diff for head
`982a47dbd55001053ed936bebf4c05a0270d943e` against live base
`befa4fda0a222f639770567ee3ac691b11ecc33f`, plus compatibility with current
master `46e1c67122085ad624c715a96a38899fe1d8e9c6`.
- Goal and correctness: cached-empty or unknown RowBinlog partitions are
refreshed from MetaService before empty-partition pruning, closing the
stale-cache window that could hide newly visible committed data.
- Concurrency: the previous Major is fixed. Each partition is now classified
by exactly one `hasDataCached()` call, so a concurrent false-to-true cache
transition cannot make it disappear from both the known-nonempty and unknown
collections. Cached-known-nonempty remains safe because visible versions are
monotonic.
- RPC and locking: RPC remains outside the ordered table version locks;
cache epochs, monotonic version/TSO updates, retry/error propagation, and
SummaryProfile accounting retain their existing contracts.
- Lifecycle and compatibility: direct `@incr`, MOW time travel,
TableBinlogFunction, fixed and unfixed Table Stream wrappers, non-Cloud mode,
and `enable_cloud_snapshot_version=false` were checked. No protocol,
persistence, configuration, or rolling-upgrade incompatibility was introduced.
The PR merges cleanly with current master.
- Tests and CI: the deterministic `false, true` Mockito sequence plus
exactly-once verification covers the old classification race. CheckStyle and
lightweight GitHub checks passed on this head; FE UT and compile have not run
on this head. No local build or test was run.
- Minor: the helper still returns cached-known-nonempty IDs before refreshed
IDs rather than preserving candidate order, causing an unnecessary scan rewrite
but no result-set error.
- Nit: both classification lists preallocate capacity for every partition,
creating one avoidable empty N-capacity backing array in common all-known or
all-unknown cases.
Findings: Blocker 0, Major 0, Minor 1, Nit 1. The review converged in one
round.
<!-- doris-repo-review:v1:begin -->
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68138
commit: 982a47dbd55001053ed936bebf4c05a0270d943e
base: befa4fda0a222f639770567ee3ac691b11ecc33f
reviewed_at: 2026-09-20T15:56:26+08:00
reviewer: luwei16
model: gpt-5.6-sol
effort: xhigh
findings: {blocker: 0, major: 0, minor: 1, nit: 1}
rounds: 1
converged: true
```
<!-- doris-repo-review:v1:end -->
--
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]
