AkkomaGang/akkoma
AkkomaGang/akkoma
15
65
Fork 92
Code Issues 177 Pull requests 25 Projects 2 Releases 14 Packages Wiki Activity

Avoid accumulation of stale data in websockets #806

Merged
floatingghost merged 1 commit from Oneric/akkoma:websocket_fullsweep into develop 2024-06-23 02:19:37 +00:00
Conversation 4 Commits 1 Files changed 1 +6
Oneric commented 2024-06-16 16:02:59 +00:00
Member
Copy link

This is the patch which came up recently again; originally reported on IRC by bjo and jmjl on IRC in March and they also tested global ERL_FULLSWEEP_AFTER. Nobody responded re testing this more specific tweak though. Given all the other pointers towards websockets it seems likely to be the culprit though.

Putting this here for more visibility or for merging anyway if you think it’s reasonably likely to be correct.
See commit message for more details.

This is the patch which came up recently again; originally reported on IRC by bjo and jmjl on IRC in March and they also tested global `ERL_FULLSWEEP_AFTER`. Nobody responded re testing this more specific tweak though. Given all the other pointers towards websockets it seems likely to be the culprit though. Putting this here for more visibility or for merging anyway if you think it’s reasonably likely to be correct. See commit message for more details.
Oneric commented 2024-06-16 16:14:03 +00:00
Author
Member
Copy link

looks like you also have accounts here @bjo @jmjl; can you test this patch and remove the global ERL_FULLSWEEP_AFTER? Ideally also verify the memleak still occurs when neither this patch is applied nor global ERL_FULLSWEEP_AFTER set.

looks like you also have accounts here @bjo @jmjl; can you test this patch and remove the global `ERL_FULLSWEEP_AFTER`? Ideally also verify the memleak still occurs when neither this patch is applied nor global `ERL_FULLSWEEP_AFTER` set.
floatingghost reviewed 2024-06-16 16:29:41 +00:00
lib/pleroma/web/mastodon_api/websocket_handler.ex Outdated
@ -61,1 +63,4 @@
# process is long-lived and can sometimes accumulate stale data in such a way it's
# not freed by young garbage cycles, thus make full collection sweeps more frequent
:erlang.process_flag(:fullsweep_after, @fullsweep_after)
floatingghost commented 2024-06-16 16:29:41 +00:00
Owner
Copy link

it looks like you shoullllld be able to specify this in ws options rather than having to use the lower-level Erlang functions

https://hexdocs.pm/phoenix/Phoenix.Endpoint.html#socket/3-websocket-configuration

it looks like you shoullllld be able to specify this in ws options rather than having to use the lower-level Erlang functions https://hexdocs.pm/phoenix/Phoenix.Endpoint.html#socket/3-websocket-configuration
Oneric commented 2024-06-17 00:32:53 +00:00
Author
Member
Copy link

We currently don’t use Phoenix’ higher level Websocket interface for this, instead WebsocketHandler is listed as a higher prio endpoint than Pleroma.Web:Endpoint. I don’t know why this is the case (maybe the higher-level interface didn’t exist yet when streaming support was introduced seven years ago) and when i wrote this patch somem onths ago i had hoped to first get confirmation whether this really is the issue. Pleroma migrated to Phoenix’ WebSockets which is why they can use this setting in their patch. If there’s no reason to stick with the custom implementation we prob should migrate as well.

There’s also lib/phoenix/transports/web_socket/raw.ex which defines a module in Phoenix’ namespace Phoenix.Transports.WebSocket.Raw but i have no idea what this does.

We currently don’t use Phoenix’ higher level Websocket interface for this, instead `WebsocketHandler` is listed as a higher prio endpoint than `Pleroma.Web:Endpoint`. I don’t know why this is the case (maybe the higher-level interface didn’t exist yet when streaming support was introduced seven years ago) and when i wrote this patch somem onths ago i had hoped to first get confirmation whether this really is the issue. Pleroma migrated to Phoenix’ WebSockets which is why they can use this setting in their patch. If there’s no reason to stick with the custom implementation we prob should migrate as well. There’s also `lib/phoenix/transports/web_socket/raw.ex` which defines a module in Phoenix’ namespace `Phoenix.Transports.WebSocket.Raw` but i have no idea what this does.
bjo commented 2024-06-18 06:22:18 +00:00
First-time contributor
Copy link

I will try go use a Source based instance the next days to test it.

I will try go use a Source based instance the next days to test it.
Oneric commented 2024-06-18 16:17:02 +00:00
Author
Member
Copy link

Thanks! When you start your from-source server, it be great if you could use

MIX_ENV=prod elixir --sname akkoma --cookie akkoma_cookie -S mix phx.server

such that — in case the patch doesn't work — you can run the memleak diagnostic tool later: diagnostic-tools/binary-leak-checker.sh

This way we’ll (hopefully) be able to get insight into which processes are actually responsible for the leaks.
Ideally test if connecting a remote shell works ahead of time; if it doesn’t you may have configured epmd too strictly, but you can just ask on IRC or here about it.

Thanks! When you start your from-source server, it be great if you could use ``` MIX_ENV=prod elixir --sname akkoma --cookie akkoma_cookie -S mix phx.server ``` such that — in case the patch doesn't work — you can run the memleak diagnostic tool later: [diagnostic-tools/binary-leak-checker.sh](https://akkoma.dev/AkkomaGang/akkoma/src/branch/develop/diagnostic-tools/binary-leak-checker.sh) This way we’ll (hopefully) be able to get insight into which processes are actually responsible for the leaks. Ideally test if connecting a remote shell works ahead of time; if it doesn’t you may have configured `epmd` too strictly, but you can just ask on IRC or here about it.
floatingghost approved these changes 2024-06-17 21:36:23 +00:00
floatingghost left a comment
Owner
Copy link

approving code-side, will need to wait to see if this fixes the issue before merge

approving code-side, will need to wait to see if this fixes the issue before merge
bjo commented 2024-06-22 07:14:21 +00:00
First-time contributor
Copy link

Unfortunately I wasn't able to package the source variant, so I'm running now a binary install with 4b5a398f22, and it seems much better now: Akkoma is using around 300-400MB memory.

Unfortunately I wasn't able to package the source variant, so I'm running now a binary install with 4b5a398f22, and it seems much better now: Akkoma is using around 300-400MB memory.
Oneric force-pushed websocket_fullsweep from 4b5a398f22 to 13e2a811ec 2024-06-22 23:22:58 +00:00 Compare
floatingghost commented 2024-06-23 02:19:32 +00:00
Owner
Copy link

feedback is good by me, we should watch out to see if migrating to the new phoenix interface is worth it or not

feedback is good by me, we should watch out to see if migrating to the new phoenix interface is worth it or not
👍 1
floatingghost merged commit f66135ed08 into develop 2024-06-23 02:19:37 +00:00
floatingghost deleted branch websocket_fullsweep 2024-06-23 02:19:37 +00:00
bjo commented 2024-06-23 20:54:39 +00:00
First-time contributor
Copy link

Memory usage is still fine, actually 350 MB running since 2024-06-21 21:32:48 CEST

Memory usage is still fine, actually 350 MB running since 2024-06-21 21:32:48 CEST
Sign in to join this conversation.
Reviewers
No reviewers
floatingghost
Labels
Clear labels   
approved, awaiting change

   
bug

Something is not working

  
configuration

This requires config changes

  
documentation

This is about human-readable docs

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
extremely low priority

   
feature request

Something new?

  
Fix it yourself

Support has been attempted

  
help wanted

Need some help

  
invalid

Something is wrong

  
mastodon_api

   
needs docs

   
needs tests

   
not a bug

   
planned

   
pleroma_api

   
privacy

   
question

More information is needed

  
static_fe

   
triage

   
wontfix

This won't be fixed

No labels
approved, awaiting change
bug
configuration
documentation
duplicate
enhancement
extremely low priority
feature request
Fix it yourself
help wanted
invalid
mastodon_api
needs docs
needs tests
not a bug
planned
pleroma_api
privacy
question
static_fe
triage
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma#806
No description provided.
Delete branch "Oneric/akkoma:websocket_fullsweep"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?