aiai-bot commented on PR #3962: URL: https://github.com/apache/hertzbeat/pull/3962#issuecomment-3744197938
<!-- code-pr-reviewer --> **Blocking issues found - recommend addressing before merge:** - **StatusPageServiceImpl.java:136-141,239-246** - Race condition in midnight timestamp calculation: multiple `Instant.now()` calls may span across midnight, causing `todayStartTimestamp > nowTimestamp` and incorrect query results. Use a single `Instant now = Instant.now()` instance for both calculations. - **StatusPageServiceImpl.java:183,304** - DST handling error: `minusSeconds(86400)` produces offset accumulation on daylight saving transition days (23/25-hour days). Use `minusDays(1)` instead to let Java Time API handle DST correctly. - **StatusPageServiceImpl.java:157** - Integer overflow risk: `(int)(o1.getTimestamp() - o2.getTimestamp())` truncates high bits for timestamp differences > ~24.8 days. Use `Long.compare(o1.getTimestamp(), o2.getTimestamp())` (already fixed correctly at line 270). - **PR description mismatch** - Claims to "add `account_expiry` metrics in `app-mysql.yml`" but no changes to this file exist in the diff. Please update PR description or add the missing file changes. **Suggested tests** (manual validation): - Test status page access at 23:59:59-00:00:01 to verify midnight boundary data - Verify with DST timezones (America/New_York, Europe/London) - Check sorting correctness for long time ranges -- 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]
