mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
04fca84872
## Background A 5000-chat scaletest (~50k turns, ~2m45s wall time) completed successfully, but the main bottleneck was **DB pool starvation from repeated reads**, not individually expensive SQL. The push/webpush path showed a few especially noisy reads: - `GetLastChatMessageByRole` for push body generation - `GetEnabledChatProviders` + `GetChatModelConfigByID` for push summary model resolution - `GetWebpushSubscriptionsByUserID` for every webpush dispatch This PR keeps the optimizations that remove those duplicate reads while leaving stream behavior unchanged. ## What changes in this PR ### 1. Reuse resolved chat state for push notifications `maybeSendPushNotification` used to re-read the last assistant message and re-resolve the chat model/provider after `runChat` had already done that work. Now `runChat` returns the final assistant text plus the already-resolved model and provider keys, and the push goroutine uses that state directly. That removes the extra push-path reads for: - `GetLastChatMessageByRole` - the second `resolveChatModel` path - the provider/model lookups that came with that second resolution ### 2. Cache webpush subscriptions during dispatch `Dispatch()` previously hit `GetWebpushSubscriptionsByUserID` on every push. A small per-user in-memory cache now avoids those repeated reads. The follow-up fix keeps that optimization correct: `InvalidateUser()` bumps a per-user generation so an older in-flight fetch cannot repopulate the cache with pre-mutation data after subscribe/unsubscribe. That preserves the cache win without letting local subscription changes be silently overwritten by stale fetch results. ## Why this is safe - The push change only reuses data already produced during the same chat run. It does not change notification semantics; if there is no assistant text to summarize, the existing fallback body still applies. - The webpush change keeps the existing TTL and `410 Gone` cleanup behavior. The generation guard only prevents stale in-flight fetches from poisoning the shared cache after invalidation. - The final PR does **not** change stream setup, pubsub/relay behavior, or chat status snapshot timing. ## Deliberately not included - No stream-path optimization in `Subscribe`. - No inline pubsub message payloads. - No distributed cross-replica webpush cache invalidation.
156 lines
4.8 KiB
Go
156 lines
4.8 KiB
Go
package coderd
|
|
|
|
import (
|
|
"database/sql"
|
|
"errors"
|
|
"net/http"
|
|
"slices"
|
|
|
|
"github.com/coder/coder/v2/coderd/database"
|
|
"github.com/coder/coder/v2/coderd/database/dbtime"
|
|
"github.com/coder/coder/v2/coderd/httpapi"
|
|
"github.com/coder/coder/v2/coderd/httpmw"
|
|
"github.com/coder/coder/v2/coderd/rbac"
|
|
"github.com/coder/coder/v2/coderd/rbac/policy"
|
|
"github.com/coder/coder/v2/coderd/webpush"
|
|
"github.com/coder/coder/v2/codersdk"
|
|
)
|
|
|
|
// @Summary Create user webpush subscription
|
|
// @ID create-user-webpush-subscription
|
|
// @Security CoderSessionToken
|
|
// @Accept json
|
|
// @Tags Notifications
|
|
// @Param request body codersdk.WebpushSubscription true "Webpush subscription"
|
|
// @Param user path string true "User ID, name, or me"
|
|
// @Router /users/{user}/webpush/subscription [post]
|
|
// @Success 204
|
|
// @x-apidocgen {"skip": true}
|
|
func (api *API) postUserWebpushSubscription(rw http.ResponseWriter, r *http.Request) {
|
|
ctx := r.Context()
|
|
user := httpmw.UserParam(r)
|
|
var req codersdk.WebpushSubscription
|
|
if !httpapi.Read(ctx, rw, r, &req) {
|
|
return
|
|
}
|
|
|
|
if err := api.WebpushDispatcher.Test(ctx, req); err != nil {
|
|
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
|
Message: "Failed to test webpush subscription",
|
|
Detail: err.Error(),
|
|
})
|
|
return
|
|
}
|
|
|
|
if _, err := api.Database.InsertWebpushSubscription(ctx, database.InsertWebpushSubscriptionParams{
|
|
CreatedAt: dbtime.Now(),
|
|
UserID: user.ID,
|
|
Endpoint: req.Endpoint,
|
|
EndpointAuthKey: req.AuthKey,
|
|
EndpointP256dhKey: req.P256DHKey,
|
|
}); err != nil {
|
|
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
|
Message: "Failed to insert push notification subscription.",
|
|
Detail: err.Error(),
|
|
})
|
|
return
|
|
}
|
|
if invalidator, ok := api.WebpushDispatcher.(webpush.SubscriptionCacheInvalidator); ok {
|
|
invalidator.InvalidateUser(user.ID)
|
|
}
|
|
|
|
rw.WriteHeader(http.StatusNoContent)
|
|
}
|
|
|
|
// @Summary Delete user webpush subscription
|
|
// @ID delete-user-webpush-subscription
|
|
// @Security CoderSessionToken
|
|
// @Accept json
|
|
// @Tags Notifications
|
|
// @Param request body codersdk.DeleteWebpushSubscription true "Webpush subscription"
|
|
// @Param user path string true "User ID, name, or me"
|
|
// @Router /users/{user}/webpush/subscription [delete]
|
|
// @Success 204
|
|
// @x-apidocgen {"skip": true}
|
|
func (api *API) deleteUserWebpushSubscription(rw http.ResponseWriter, r *http.Request) {
|
|
ctx := r.Context()
|
|
user := httpmw.UserParam(r)
|
|
|
|
var req codersdk.DeleteWebpushSubscription
|
|
if !httpapi.Read(ctx, rw, r, &req) {
|
|
return
|
|
}
|
|
|
|
// Return NotFound if the subscription does not exist.
|
|
existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID)
|
|
if err != nil {
|
|
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
|
Message: "Failed to get webpush subscriptions.",
|
|
Detail: err.Error(),
|
|
})
|
|
return
|
|
}
|
|
if idx := slices.IndexFunc(existing, func(s database.WebpushSubscription) bool {
|
|
return s.Endpoint == req.Endpoint
|
|
}); idx == -1 {
|
|
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
|
|
Message: "Webpush subscription not found.",
|
|
})
|
|
return
|
|
}
|
|
|
|
if err := api.Database.DeleteWebpushSubscriptionByUserIDAndEndpoint(ctx, database.DeleteWebpushSubscriptionByUserIDAndEndpointParams{
|
|
UserID: user.ID,
|
|
Endpoint: req.Endpoint,
|
|
}); err != nil {
|
|
if errors.Is(err, sql.ErrNoRows) {
|
|
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
|
|
Message: "Webpush subscription not found.",
|
|
})
|
|
return
|
|
}
|
|
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
|
Message: "Failed to delete push notification subscription.",
|
|
Detail: err.Error(),
|
|
})
|
|
return
|
|
}
|
|
if invalidator, ok := api.WebpushDispatcher.(webpush.SubscriptionCacheInvalidator); ok {
|
|
invalidator.InvalidateUser(user.ID)
|
|
}
|
|
|
|
rw.WriteHeader(http.StatusNoContent)
|
|
}
|
|
|
|
// @Summary Send a test push notification
|
|
// @ID send-a-test-push-notification
|
|
// @Security CoderSessionToken
|
|
// @Tags Notifications
|
|
// @Param user path string true "User ID, name, or me"
|
|
// @Success 204
|
|
// @Router /users/{user}/webpush/test [post]
|
|
// @x-apidocgen {"skip": true}
|
|
func (api *API) postUserPushNotificationTest(rw http.ResponseWriter, r *http.Request) {
|
|
ctx := r.Context()
|
|
user := httpmw.UserParam(r)
|
|
|
|
// We need to authorize the user to send a push notification to themselves.
|
|
if !api.Authorize(r, policy.ActionCreate, rbac.ResourceNotificationMessage.WithOwner(user.ID.String())) {
|
|
httpapi.Forbidden(rw)
|
|
return
|
|
}
|
|
|
|
if err := api.WebpushDispatcher.Dispatch(ctx, user.ID, codersdk.WebpushMessage{
|
|
Title: "It's working!",
|
|
Body: "You've subscribed to push notifications.",
|
|
}); err != nil {
|
|
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
|
Message: "Failed to send test notification",
|
|
Detail: err.Error(),
|
|
})
|
|
return
|
|
}
|
|
|
|
rw.WriteHeader(http.StatusNoContent)
|
|
}
|