dataroaring commented on PR #59539: URL: https://github.com/apache/doris/pull/59539#issuecomment-3774218734
### Code review Found 2 issues: 1. **Per-replica counting instead of per-tablet counting** - The `TabletSlidingWindowAccessStats.recordTablet()` call is placed inside lambdas that execute once per replica, causing tablet access to be counted N times (where N = number of replicas) instead of once per tablet access. For a tablet with 3 replicas, this means 3x overcounting, which will skew the active tablet statistics used by the rebalancer. https://github.com/apache/doris/blob/0dbd4cfb060a12c1ae84fe6652a1ebe63bc929e2/fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java#L247-L252 The same issue exists in CloudTablet: https://github.com/apache/doris/blob/0dbd4cfb060a12c1ae84fe6652a1ebe63bc929e2/fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTablet.java#L67-L72 The correct pattern (recording outside the per-replica loop) is shown in `getQueryableReplicas()`: https://github.com/apache/doris/blob/0dbd4cfb060a12c1ae84fe6652a1ebe63bc929e2/fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java#L264-L267 2. **Division by zero when config < 60** - If `active_tablet_sliding_window_time_window_second` is configured to a value less than 60 (DEFAULT_BUCKET_SIZE_SECOND), `numBuckets` will be 0, causing `ArithmeticException` in `getBucketIndex()` when computing `% numBuckets`. The config is mutable at runtime and has no minimum value validation. https://github.com/apache/doris/blob/0dbd4cfb060a12c1ae84fe6652a1ebe63bc929e2/fe/fe-core/src/main/java/org/apache/doris/catalog/TabletSlidingWindowAccessStats.java#L294-L297 The modulo operation that will fail: https://github.com/apache/doris/blob/0dbd4cfb060a12c1ae84fe6652a1ebe63bc929e2/fe/fe-core/src/main/java/org/apache/doris/catalog/TabletSlidingWindowAccessStats.java#L182-L184 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- 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]
