perf(webpush): Improve performance of webpush notifications - #3244
Open
nickvergessen wants to merge 4 commits into
Open
perf(webpush): Improve performance of webpush notifications#3244nickvergessen wants to merge 4 commits into
nickvergessen wants to merge 4 commits into
Conversation
webPushDeleteToDevice() queued the delete payloads on the web push client but then flushed the push proxies instead, so the queued web push deletes leaked into an unrelated later flush. Also widen the proxy rejection handler to \Throwable: Guzzle rejection reasons are not guaranteed to be \Exception, and a non-Exception reason would raise a TypeError inside the promise handler. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Joas Schilling <coding@schilljs.com>
Sending a web push notification blocks the process it runs in, so the library default of a 30 seconds timeout per request is far too long. Also pass the batch size and request concurrency explicitly instead of relying on the library defaults, and hand the library a PSR-3 logger so it stops falling back to trigger_error(). Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Joas Schilling <coding@schilljs.com>
Encrypting the payload for every subscription happened for the whole batch before the first request was sent, so the CPU time was spent inside the blocking window instead of overlapping the network I/O. PooledWebPush feeds the pool a generator instead, so the payload of the next subscription is encrypted while the previous requests are in flight, and it returns the promises of the pools instead of waiting for them. WebPushClient::flush() waits for them right away, so all existing call sites keep behaving as before. As a side effect an encryption error of a single subscription does not abort the requests of all the other subscriptions of the batch anymore. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Joas Schilling <coding@schilljs.com>
Push notifications were sent inline in whatever process triggered them, so a web request could not finish before every push endpoint and push proxy had answered. The requests are now started, but the waiting is handed to the new DeferredFlusher: on the command line it waits right away, so the output of `occ notification:test-push` stays in order and a cron run does not pile up the push traffic at the end. For web requests the waiting is postponed to the shutdown of the request, where the response is sent to the client first, when the SAPI supports it (PHP-FPM and LiteSpeed, but not mod_php). The callbacks that delete expired subscriptions and unknown devices run at shutdown as well. The shutdown functions of the server neither close the database connection nor the session, and PHP runs them in registration order, so the database and the logger are still available. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Joas Schilling <coding@schilljs.com>
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.
🤖 AI (if applicable)