Skip to content

perf(response): Flush requests early to clients - #57991

Open
nickvergessen wants to merge 2 commits into
masterfrom
perf/noid/flush-response-early
Open

perf(response): Flush requests early to clients#57991
nickvergessen wants to merge 2 commits into
masterfrom
perf/noid/flush-response-early

Conversation

@nickvergessen

@nickvergessen nickvergessen commented Feb 2, 2026

Copy link
Copy Markdown
Member

If not, the response will wait for async actions, e.g. HTTP requests from IClientService, to be finished before returning. This is not desired in many cases with e.g. with notifications.

Checklist

@nickvergessen nickvergessen added this to the Nextcloud 34 milestone Feb 2, 2026
@nickvergessen nickvergessen self-assigned this Feb 2, 2026
@nickvergessen
nickvergessen requested a review from a team as a code owner February 2, 2026 10:45
@nickvergessen
nickvergessen requested review from ArtificialOwl, icewind1991, leftybournes and salmart-dev and removed request for a team February 2, 2026 10:45
@nickvergessen
nickvergessen marked this pull request as draft February 2, 2026 10:45
Altahrim
Altahrim reviewed Feb 10, 2026
View reviewed changes

@Altahrim Altahrim left a comment •
edited by nickvergessen
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments, but I like the idea :)

There is also fastcgi_finish_request for FPM, it may worth a try as it also kills the connection

Comment thread lib/private/AppFramework/App.php Outdated
Comment thread lib/public/AppFramework/Http/Response.php
@nickvergessen

nickvergessen commented Feb 10, 2026

Copy link
Copy Markdown
Member Author

From PHP comments a small reminder:

There are some pitfalls you should be aware of when using this function.
...
Another important thing is session handling. Sessions are locked as long as they're active (see the documentation for session_write_close()). This means subsequent requests will block until the session is closed.

You should therefore call session_write_close() as soon as possible (even before fastcgi_finish_request()) to allow subsequent requests and a good user experience.

We should ensure session was closed before.

@Altahrim

Altahrim commented Feb 12, 2026

Copy link
Copy Markdown
Collaborator

Good remark about session.

I think one of the best scenario would be:

  • open session
  • generate HTTP headers
  • flush
  • generate <head>
  • close_session
  • flush // At this point, browser can already load all related resources (CSS, JS…)
  • generate content (all info about users have already been loaded)
  • flush / end-request
  • do post query stuff not needed by client

I didn't check what is possible. It may be possible to reduce session access even more

@juliusknorr

juliusknorr commented Feb 17, 2026

Copy link
Copy Markdown
Member

Regarding reducing session access, #32162 is probably interesting. Back then we introduced a config flag to close the session early and reopen if needed, but it has impact on the session expiry which can then not be as reliably as before anymore.

@susnux susnux modified the milestones: Nextcloud 34.0.1, Nextcloud 35 Jun 9, 2026
@nickvergessen
perf(response): Flush requests early to clients
811a53c 
If not, the response will wait for async actions, e.g. HTTP requests from
IClientService, to be finished before returning. This is not desired in
many cases with e.g. with notifications.

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
perf(response): Handle different SAPIs better and document implications
677634f 
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessen force-pushed the perf/noid/flush-response-early branch from d4ce68e to 677634f Compare July 31, 2026 13:56
@nickvergessen
nickvergessen requested a review from Altahrim July 31, 2026 13:56
@nickvergessen
nickvergessen marked this pull request as ready for review July 31, 2026 13:58
@nickvergessen

nickvergessen commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Rebased and further discussed that with my coding peer (Claude).
It's just the base work for now each part that could benefit from it needs to do that manually:

  • Use async when sending requests (webhooks, guzzle, federation needs some rework here, …)
  • Don't flush notifications early (so notifications app or something else can do it in the code after it)
  • If that is all working, the app can opt-in into the "flush early"

AndyScherzinger
AndyScherzinger approved these changes Jul 31, 2026
View reviewed changes

@AndyScherzinger AndyScherzinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes
@SystemKeeper SystemKeeper SystemKeeper approved these changes
@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl is a code owner automatically assigned from nextcloud/server-backend
@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend
@salmart-dev salmart-dev Awaiting requested review from salmart-dev salmart-dev is a code owner automatically assigned from nextcloud/server-backend
@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend
@Altahrim Altahrim Awaiting requested review from Altahrim
@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst
@sudormant sudormant Awaiting requested review from sudormant

Assignees

@nickvergessen nickvergessen

Labels

3. to review Waiting for reviews AI assisted performance 🚀 technical debt 🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 35

Development

Successfully merging this pull request may close these issues.

9 participants

@nickvergessen @Altahrim @juliusknorr @AndyScherzinger @SystemKeeper @joshtrichards @susnux @nextcloud-bot @nextcloud-command