search

LiamMorrow / OrgnalR

SignalR backplane implemented through Orleans
Apache License 2.0
35 stars 8 forks source link

[BUG] Potential memory leak #64

Open koenbeuk opened 1 year ago

koenbeuk commented 1 year ago

We're using this library in production for sending non-critical signals to users. Over time we notice a degradation in performance and an increase in memory consumption until at some point we get an application restart with an OOM message.

We suspect the problem is with RewindableMessageGrain. As its associated to Clients (which come and go) and persists messages in state, State however is not cleared when the client goes away and will always stay persisted.

2 relatively easy wins would be to never persist when MaxMessageRewind is configured as 0 and to make persistence optional. A proper fix would involve observing the client and clearing state when the client goes away.

We're using MemoryStorage which exacerbates the issue. If we were to use a proper storage provider then we would expect to see an ever increasing db size.

I'm happy to contribute here.

LiamMorrow commented 1 year ago

Ah yeah that's definitely an issue, I'd say even (to a lesser extent) even with the User and Group grains which contain a hashset of the connectionIds. It's potentially worth having some sort of TTL since last accessed, and a monitor grain which clears the state for each entity.

koenbeuk commented 1 year ago

Opened up a PR that would partly solve this issue, I agree that it's not completely solved and that we need some TTL mechanism. Would really appreciate it if the PR could be accepted and a new version could be released.

LiamMorrow commented 1 year ago

Thanks for that! Gave it a test, and seems to work without issue, releasing 2.3.1 now