Skip to content

Commit 42c4ada

Browse files
Optimize notification unread polling (#987)
## Summary - Gate `limit=0` notification unread polling behind the fast single-recipient path before touching the broader multi-recipient GIN scan. - Add a concurrent partial GIN index for multi-recipient `notification.user_ids` rows. - Cover capped unread count and single/multi group dedupe behavior in tests. ## Live analysis - Current bridge reads; replication lag was ms-level during the investigation. - Remaining `/v1/notifications/:userId` 504s were low-volume but concentrated in a few users, with requests timing out at the 8s notification read deadline. - Read-only EXPLAIN against the replica for top user `DvEvv`/`77214` improved the unread poll shape from roughly 132ms / 39,651 buffer hits to roughly 14ms / 1,132 buffer hits by avoiding the multi-recipient branch when the single-recipient branch already satisfied the capped unread count. - User `eJ57D`/`51` showed the same branch-elision behavior. - Residual users with fewer than 20 single-recipient unread groups still need the fallback, so the partial multi-recipient GIN index targets that path. ## Testing - `go test ./api -run '^$'` - `git diff --check` Note: focused notification integration tests could not run locally because the local test Postgres at `localhost:21300` was not running.
1 parent e2849c8 commit 42c4ada

3 files changed

Lines changed: 104 additions & 6 deletions

File tree

api/v1_notifications.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ WITH latest_seen AS (
7474
-- Equivalent to ARRAY[@user_id] && n.user_ids, split so single-recipient rows
7575
-- can use notification_single_recipient_user_timestamp_idx while
7676
-- multi-recipient arrays keep the existing overlap semantics.
77-
matched_notifications AS (
77+
single_recipient_groups AS MATERIALIZED (
7878
SELECT n.type, n.group_id
7979
FROM notification n
8080
CROSS JOIN latest_seen
@@ -84,22 +84,40 @@ matched_notifications AS (
8484
AND n.timestamp > COALESCE(latest_seen.seen_at, '-infinity'::timestamp)
8585
AND (n.type = ANY(@types) OR @types IS NULL)
8686
AND (n.type != ALL(@unsupported_types))
87-
UNION ALL
87+
GROUP BY n.type, n.group_id
88+
LIMIT @limit
89+
),
90+
single_recipient_count AS MATERIALIZED (
91+
SELECT COUNT(*)::int AS unread_count
92+
FROM single_recipient_groups
93+
),
94+
-- Only touch the broader multi-recipient GIN path if the fast single-recipient
95+
-- path did not already satisfy the capped unread count.
96+
multi_recipient_groups AS MATERIALIZED (
8897
SELECT n.type, n.group_id
8998
FROM notification n
9099
CROSS JOIN latest_seen
91-
WHERE COALESCE(array_length(n.user_ids, 1), 0) != 1
100+
WHERE (SELECT unread_count FROM single_recipient_count) < @limit
101+
AND COALESCE(array_length(n.user_ids, 1), 0) != 1
92102
AND ARRAY[@user_id] && n.user_ids
93103
AND n.timestamp > (now()::timestamp - interval '90 days')
94104
AND n.timestamp > COALESCE(latest_seen.seen_at, '-infinity'::timestamp)
95105
AND (n.type = ANY(@types) OR @types IS NULL)
96106
AND (n.type != ALL(@unsupported_types))
107+
AND NOT EXISTS (
108+
SELECT 1
109+
FROM single_recipient_groups sg
110+
WHERE sg.type = n.type
111+
AND sg.group_id = n.group_id
112+
)
113+
GROUP BY n.type, n.group_id
114+
LIMIT GREATEST(@limit - (SELECT unread_count FROM single_recipient_count), 0)
97115
)
98116
SELECT COUNT(*)
99117
FROM (
100-
SELECT 1
101-
FROM matched_notifications
102-
GROUP BY type, group_id
118+
SELECT type, group_id FROM single_recipient_groups
119+
UNION ALL
120+
SELECT type, group_id FROM multi_recipient_groups
103121
LIMIT @limit
104122
) unread_notifications;
105123
`

api/v1_notifications_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,72 @@ func TestV1Notifications_ReturnsMultiRecipientRows(t *testing.T) {
152152
})
153153
}
154154

155+
func TestV1Notifications_LimitZeroDedupesSingleAndMultiRecipientGroups(t *testing.T) {
156+
app := emptyTestApp(t)
157+
158+
fixtures := database.FixtureMap{
159+
"notification": []map[string]any{
160+
{
161+
"id": 1,
162+
"specifier": "single",
163+
"group_id": "milestone:shared",
164+
"type": "milestone",
165+
"user_ids": []int{1},
166+
"timestamp": time.Now().Add(-1 * time.Minute),
167+
"data": []byte(`{"type": "TRACK_REPOST_COUNT", "threshold": 10, "track_id": 101}`),
168+
},
169+
{
170+
"id": 2,
171+
"specifier": "multi",
172+
"group_id": "milestone:shared",
173+
"type": "milestone",
174+
"user_ids": []int{1, 2},
175+
"timestamp": time.Now(),
176+
"data": []byte(`{"type": "TRACK_SAVE_COUNT", "threshold": 10, "track_id": 102}`),
177+
},
178+
},
179+
}
180+
181+
database.Seed(app.pool.Replicas[0], fixtures)
182+
183+
status, body := testGet(t, app, "/v1/notifications/"+trashid.MustEncodeHashID(1)+"?limit=0")
184+
assert.Equal(t, 200, status)
185+
186+
jsonAssert(t, body, map[string]any{
187+
"data.notifications.#": 0,
188+
"data.unread_count": 1,
189+
})
190+
}
191+
192+
func TestV1Notifications_LimitZeroCapsUnreadCount(t *testing.T) {
193+
app := emptyTestApp(t)
194+
195+
notifs := make([]map[string]any, 0, notificationUnreadPollLimit+1)
196+
for i := range notificationUnreadPollLimit + 1 {
197+
notifs = append(notifs, map[string]any{
198+
"id": i + 1,
199+
"specifier": strconv.Itoa(i + 1),
200+
"group_id": "milestone:" + strconv.Itoa(i+1),
201+
"type": "milestone",
202+
"user_ids": []int{1},
203+
"timestamp": time.Now().Add(-1 * time.Duration(i) * time.Minute),
204+
"data": []byte(`{"type": "TRACK_REPOST_COUNT", "threshold": 10, "track_id": 101}`),
205+
})
206+
}
207+
208+
database.Seed(app.pool.Replicas[0], database.FixtureMap{
209+
"notification": notifs,
210+
})
211+
212+
status, body := testGet(t, app, "/v1/notifications/"+trashid.MustEncodeHashID(1)+"?limit=0")
213+
assert.Equal(t, 200, status)
214+
215+
jsonAssert(t, body, map[string]any{
216+
"data.notifications.#": 0,
217+
"data.unread_count": notificationUnreadPollLimit,
218+
})
219+
}
220+
155221
func TestV1Notifications_NotDeletedTrack(t *testing.T) {
156222
app := emptyTestApp(t)
157223

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-- The notification API handles the overwhelmingly common single-recipient path
2+
-- with notification_single_recipient_user_timestamp_idx. Keep the fallback
3+
-- multi-recipient overlap scan off the all-row GIN index so high-notification
4+
-- users do not recheck large numbers of single-recipient rows.
5+
--
6+
-- NOTE: intentionally NOT wrapped in BEGIN/COMMIT so CREATE INDEX
7+
-- CONCURRENTLY can run without holding an ACCESS EXCLUSIVE lock on notification.
8+
9+
CREATE INDEX CONCURRENTLY IF NOT EXISTS notification_multi_recipient_user_ids_idx
10+
ON public.notification USING gin (user_ids)
11+
WHERE COALESCE(array_length(user_ids, 1), 0) != 1;
12+
13+
COMMENT ON INDEX public.notification_multi_recipient_user_ids_idx IS
14+
'Covers notification reads for the uncommon multi-recipient user_ids array path.';

0 commit comments

Comments
 (0)