github-actions[bot] commented on PR #62258: URL: https://github.com/apache/doris/pull/62258#issuecomment-4211511736
No issues found in the code changes. Review summary: - Goal / correctness: The patch fixes the reported cross-thread `CLOCK_THREAD_CPUTIME_ID` misuse by keeping CPU timer stop/update on the worker thread and using `start_queue_wait()` from the scheduler thread only. The changed paths cover both requeue and EOS handling. - Scope / minimality: The change is small and focused to the affected scanner timing paths. - Concurrency: Applicable and checked. The timer lifecycle is now thread-consistent; no new lock scope, lock-order, or atomic-order issue was introduced in the modified code. - Lifecycle / initialization: Applicable and checked. Scanner timer state remains balanced across submit, execute, requeue, and close; no static initialization concern is involved. - Config / compatibility: Not applicable. No config, protocol, or storage compatibility change. - Parallel paths: Applicable and checked. The relevant submit and `_scanner_scan()` paths were updated consistently for non-EOS and EOS cases. - Special conditions: Applicable and checked. The new comment explains the `CLOCK_THREAD_CPUTIME_ID` same-thread requirement clearly enough. - Test coverage: No automated test was added, so residual risk remains around scheduler/profile accounting regressions, but I did not find a code path in this patch that is clearly incorrect. - Observability: Existing counters/profile paths remain sufficient; no extra metrics appear necessary. - Transaction / persistence / FE-BE variable passing / data write correctness: Not applicable. - Performance: No meaningful regression found; the extra helper is trivial and avoids the prior bogus CPU accumulation. - Other issues: None found during this review. -- 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]
