Skip to content

perf(webpush): Improve performance of webpush notifications - #3244

Open
nickvergessen wants to merge 4 commits into
masterfrom
techdebt/noid/webpush-async
Open

perf(webpush): Improve performance of webpush notifications#3244
nickvergessen wants to merge 4 commits into
masterfrom
techdebt/noid/webpush-async

Conversation

@nickvergessen

@nickvergessen nickvergessen commented Jul 28, 2026

Copy link
Copy Markdown
Member

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

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>
@nickvergessen nickvergessen added this to the Nextcloud 35 milestone Jul 28, 2026
@nickvergessen nickvergessen self-assigned this Jul 28, 2026
Base automatically changed from techdebt/noid/push-proxy-async to master July 28, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant