Repository navigation
perf(notifications): cap the notification counts (#654) - #697
Merged
Merged
Conversation
GET /api/v1/notifications counted every notification of the user on each call for `total` (about 6,000 row reads per call in the cleanup-notify load test). The count now stops after offset + 1001 rows, through a LIMIT in a derived table (models.NotificationTotalCap = 1000): - up to offset + 1000 matching rows, `total` is exact and `total_capped` is false; - above that, `total` is offset + 1000 and `total_capped` is true. `unread_count` stays exact. With unread_only=true and an exact total, the handler uses the total and skips CountUnread. The window saturates at math.MaxInt64 for a huge offset. On MySQL 8.4 with 2,500 rows of one user, the count reads 1,001 covering index rows instead of 2,500 (TestDatabaseMySQL_NotificationTotalCap logs EXPLAIN, EXPLAIN ANALYZE and the handler read counters). Swagger and the agent docs describe the new field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Notifications page shows "1–25 of 1000+" when the API sets `total_capped`, and keeps paging forward. A response without the field (an older backend) counts as not capped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CountUnread read every unread row of the user on each list call and on
each GET /notifications/count. It now uses the windowed count of the list
total: exact up to 1000 unread notifications (at most 1001 rows read),
else 1000 with `unread_count_capped: true`.
- GET /notifications adds `unread_count_capped`.
- GET /notifications/count returns models.UnreadNotificationCount
({unread_count, unread_count_capped}) instead of a map.
- With unread_only=true at offset 0, the list uses its total as the unread
count and skips CountUnread. At a larger offset the total can be exact
above the cap, so the handler counts again: the list and /count always
agree.
cleanup-notify load test (k6-stress.js, 20 VUs, 5 min, one user with up to
11,605 notifications, MySQL 8.4.4, fresh database each run):
| | origin/main | this branch |
|---|---|---|
| notifications row reads | 34.6 million | 7.0 million |
| list total count, rows per call | 5,803 | 966 |
| list total count, total time | 21.2 s | 5.2 s |
The unread count stayed at about 120 rows per call in both runs, because
the scenario marks all read every fifth iteration. The branch run predates
the offset-0 shortcut change, which does not affect that scenario.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Notifications page shows "Unread (1000+)" when the API sets `unread_count_capped`. Reading one notification keeps the "+" (999+), and "Mark all read" clears it. A response without the field counts as not capped. The bell badge already shows "99+" from 100 on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #654
Summary
GET /api/v1/notificationsread every notification of the user on each call to compute the exacttotal, and every unread one forunread_count. Both counts now stop early.totalis exact up to offset + 1000. Above that it is offset + 1000 andtotal_cappedistrue. The count reads at most offset + 1001 index rows, through aLIMITin a derived table (countWindow).unread_count(list andGET /notifications/count) is exact up to 1000. Above that it is 1000 andunread_count_cappedistrue./countalways agree.The index
(user_id, created_at DESC)and the definition-lock fix of this issue are already on main (#659).API change
total,unread_countandtotal_capped/unread_count_cappedare always present. Existing clients keep working: below the caps the numbers mean what they did before, and above them they are a lower bound.GET /notifications/countnow returns{"unread_count": N, "unread_count_capped": false}(Swaggermodels.UnreadNotificationCount) instead of a map of integers. A client generated from the old Swagger that decodes into a map of integers fails on the boolean. The web UI and stackctl handle it.Load test
cleanup-notify(k6-stress.js, 20 VUs, 5 min, one user with up to 11,605 notifications, MySQL 8.4.4, a fresh database each run):notificationsBoth runs: 5,700 iterations, 24,180 requests, no failures. The unread count stayed at about 120 rows per call in both runs, because the scenario marks all read every fifth iteration.
EXPLAIN ANALYZE(2,500 rows of one user): the new count is a covering index lookup withLimit: 1001, so it reads 1,001 index rows instead of 2,500.TestDatabaseMySQL_NotificationTotalCaplogs it and checks the bound.Tests
go vet ./...(also-tags integration),go test ./... -short, the MySQL integration tests ininternal/database,make gosec(0 issues),npm test(1,972 pass) andnpm run lint(0 errors, 5 existing warnings).🤖 Generated with Claude Code