lizhimins commented on PR #5754: URL: https://github.com/apache/rocketmq-dashboard/pull/5754#issuecomment-6094438999
> Paging the replay is the right call and the new test does go red without it. Three asks before merge: > (1) this must land **after** #5753 — on its own it keeps `registry.attach` after the replay, and the > loop widens the very gap #5753 closes from one query to up to 25; (2) when the `MAX_REPLAY_PAGES` > ceiling is hit the loop stops silently, please add a `log.warn` so a truncated backlog is visible; > (3) `cursor = row.getSeq()` assumes the repository returns rows in ascending seq — please confirm the > mapper's `ORDER BY`, otherwise the cursor can move backwards and burn all 25 pages. Minor: the class > has 23 tests on the base, so the post-change total is 24. Also, your PR body contains literal `\n` > sequences and does not render on GitHub. -- 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]
