You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/10/09 21:03:06 UTC
[PR] fix(audit): disclose a cleanup that hit its per-call batch ceiling [rocketmq-dashboard]
unbridled-41 opened a new pull request, #5977:
URL: https://github.com/apache/rocketmq-dashboard/pull/5977
### Which Issue(s) This PR Fixes
Fixes #5976
### Problem / Evidence
```java
// AuditService.cleanupLogs (before)
return auditRepository.deleteBefore(cutoff, CLEANUP_BATCH_SIZE, CLEANUP_MAX_BATCHES);
```
```ts
// audit.tsx (before)
await cleanupAuditLogs(cleanupDays);
message.success(t('audit.cleanupSuccess', { n: cleanupDays }));
```
```
AuditPage: discloses a cleanup that hit the per-call cap
-> FAIL before the change (the success toast is shown)
```
### Root cause / Fix
A bounded sweep exposed as an unbounded result, and the count discarded in the view. Return `deleted` + `truncated` (a full sweep may have left expired rows), thread it through the response and the web client, and warn with the real count; a non-truncated cleanup keeps the existing message.
### Priority and scoring
**PRIORITY 47** - impact 16/40 (retention silently not applied, with the count dropped on the floor), blast radius 10/20 (any deployment above the cap), reproducibility 18/20 (pinned by tests), maintenance value 3/20. **FIX_CONFIDENCE 80**.
### Tests
- `cd server && mvn -o -B -ntp test -Dtest='AuditServiceTest,AuditControllerTest'` -> `Tests run: 32, Failures: 0, Errors: 0` (new ceiling case; the bounded-batch case now asserts `truncated=false`; three controller cases updated to the new return type).
- `cd web && npx vitest run src/pages/ops/__tests__/AuditPage.test.tsx` -> `Tests 12 passed` (the new case fails before the change).
- `npx eslint`, `npx tsc -b` and `checkstyle:check` pass.
### Risk
One extra field and one extra warning path; the non-truncated behaviour is unchanged.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(audit): disclose a cleanup that hit its per-call batch ceiling [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5977:
URL: https://github.com/apache/rocketmq-dashboard/pull/5977#issuecomment-6091395294
### Verification on the final commit (branch + commit + command results)
Every command from the description re-run on a fresh worktree of this branch (`git worktree add <dir> fork/<branch>`, no local modifications), at commit `0e7dfd04de`:
| command | exit | result |
| --- | --- | --- |
| `cd server && mvn -o -B -ntp test -Dtest='AuditServiceTest,AuditControllerTest'` | 0 | `Tests run: 32, Failures: 0, Errors: 0, Skipped: 0 · BUILD SUCCESS` |
| `cd web && npx vitest run src/pages/ops/__tests__/AuditPage.test.tsx` | 0 | `Test Files 1 passed (1) · Tests 12 passed (12)` |
CI note: on this repository the `Backend Build` (binary-license-gate), `Frontend Build` / `Frontend Docker Build` (licence manifest) and `rmqctl Build` (YAML renderer quoting) jobs are red for every branch, trunk included; the module-level commands above are unaffected.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(audit): disclose a cleanup that hit its per-call batch ceiling [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #5977: fix(audit): disclose a cleanup that hit its per-call batch ceiling
URL: https://github.com/apache/rocketmq-dashboard/pull/5977
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(audit): disclose a cleanup that hit its per-call batch ceiling [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5977:
URL: https://github.com/apache/rocketmq-dashboard/pull/5977#issuecomment-6227219710
This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated changes.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(audit): disclose a cleanup that hit its per-call batch ceiling [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5977:
URL: https://github.com/apache/rocketmq-dashboard/pull/5977#issuecomment-6090238417
### CI status of this PR
`Backend Tests (Java 21)` — see the note below the list.
The four remaining red jobs are red for every branch, including `rocketmq-studio` itself:
- `Backend Build (Java 21)` — the `binary-license-gate` antrun step fails on a list of third-party jars (`spring-ai-*`, `swagger-ui`, `tencentcloud-sdk-*`, `zstd-jni`, ... : "missing complete upstream LICENSE/COPYING; a POM name or link is not a substitute"). This PR touches no build configuration; tracked by #5341.
- `Frontend Build (Node 20)` / `Frontend Docker Build` — the frontend licence manifest check fails the same way on branches that do not touch `web/` at all; tracked by #5346.
- `rmqctl Build (Go)` — `cmd` `TestMessageQueryCompletenessWarningTest` yaml subtests: `got {"items":[{"size":"3",...}]}, want {"size":3,...}`, i.e. the YAML renderer writes numbers as quoted strings where the test expects integer literals. Reproduced locally with `cd rmqctl && go test ./cmd/` from a checkout whose `rmqctl/` tree is byte-identical to `rocketmq-studio`'s, and owned by open PR #5850 ("keep yaml numbers unquoted").
`Backend Tests (Java 21)` failure attribution: `ResourceOwnershipGuardTest.migrationConcurrentStartupIsIdempotentTest` — `org.h2.jdbc.JdbcSQLNonTransientException: General error: java.util.ConcurrentModificationException` — the known flaky H2 concurrency test tracked by #5302. It reproduces independently of this diff (which touches no persistence code) and the same job passes on sibling PRs of the same batch.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(audit): disclose a cleanup that hit its per-call batch ceiling [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5977:
URL: https://github.com/apache/rocketmq-dashboard/pull/5977#issuecomment-6091111198
### Follow-up review of this fix: the warning claimed more than the flag can prove
`truncated` is computed as `deleted >= 500 * 20` — the sweep ending on its ceiling. That is a *"rows may remain"* signal, not proof: a cluster with exactly 10,000 expired rows deletes all of them and still gets the flag. The warning asserted the rows were still there, so in that case the operator is sent to run a cleanup again for nothing.
Commit `0e7dfd04` aligns the wording with what the flag can prove (first commit `82f19d77` untouched, no force-push):
- `audit.cleanupTruncated` now reads `…，可能仍有 {days} 天之前的记录…` / `…but older entries may remain…` instead of stating that they remain;
- the `AuditService` comment (and the twin comment in `AuditServiceTest.cleanupLogsReportsWhenTheSweepHitItsCeilingTest`) records why the flag is a possibility and the message has to say so.
The rest of the PR is unchanged: complete sweeps still keep the existing success wording, and the `deleted` count is still reported.
Verified locally:
```
cd server && mvn -o -B -ntp test -Dtest='AuditServiceTest,AuditControllerTest'
Tests run: 32, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
cd web && npx vitest run src/pages/ops/__tests__/AuditPage.test.tsx
Test Files 1 passed (1)
Tests 12 passed (12)
```
`npx tsc -b` and `npx eslint` on the touched files are clean.
Branch `fix/audit-cleanup-truncation`, HEAD `0e7dfd04` (on top of `82f19d77`).
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org