search

metarhia / impress

Enterprise application server for Node.js and Metarhia private cloud ⚡
https://metarhia.com
MIT License
970 stars 129 forks source link

Memory leaks #892

Closed ykolomiets closed 5 years ago

ykolomiets commented 5 years ago

Hi, guys! We use RPC server, and after working several hours there are 1.5GB retained strings, 99% of them are packets ("{event:[-79,'test'],payload:[{abc:5}]}"). I have found that object which retains all these strings is sessionsStorage image We don't need sessions at all, so how can I disable this feature? Thanks in advance)

tshemsedinov commented 5 years ago

It looks like JSTP server leaks. Please @belochub check this

belochub commented 5 years ago

@ykolomiets, do you have heartbeat enabled on your server? This seems to be the issue with the server saving the sent messages in case they are not delivered and not being able to delete them because no response messages (callback, pong) are being received from the client side (event messages are not considered to be the response messages). I wouldn't recommend disabling sessions, and I don't think there is a way to do it since they are deeply interconnected with jstp internals. Sessions are part of the protocol, and they fulfill the promise that all of the created initiating messages (e.g. event, call) will be successfully transmitted over the network as soon as possible. You don't have to send heartbeat messages too often if you want to avoid the session buffer using up too much space, I think, sending heartbeat messages every 1-5 minutes should be enough. And in case you have your heartbeats enabled already, such a problem can occur if you are trying to send a lot of messages to closed connections and they are being buffered instead of being transmitted over the network. In this case, you should try to limit the number of messages you send via the closed connections.

nechaido commented 5 years ago

@ykolomiets Also it looks like it may be some kind of test leftover, interface test and event name payload.

Though it can be that these events are created intentionally to simulate a workload if that is the case adressing @belochub's https://github.com/metarhia/impress/issues/892#issuecomment-437976406 should help you.

ykolomiets commented 5 years ago

@belochub Ok, heartbeat is disabled now, so I'm going to enable it and collect new data. I think this kind of things must be written up in docs, isn't it? Thanks, I will write a new comment tomorrow.

ykolomiets commented 5 years ago

In this case, you should try to limit the number of messages you send via the closed connections.

If a connection has been closed and I have removed all links on that connection in my environment, will it be collected by GC? Even if there are some not delivered messages?

nechaido commented 5 years ago

@ykolomiets

In this case, you should try to limit the number of messages you send via the closed connections.

If a connection has been closed and I have removed all links on that connection in my environment, will it be collected by GC? Even if there are some not delivered messages?

Connections are stored in an ExpiringMap that will automatically remove them after one hour of being closed. Sessions are stored in a StorageProvider that will remove sessions after (24-25) hours of being inactive. At the moment there is no way to change these options in impress.

ykolomiets commented 5 years ago

@tshemsedinov @belochub I think that impress doesn't use 'heartbeat' property in config/servers.js! Is it bug?

belochub commented 5 years ago

@ykolomiets, I think yes, we have changed the option name to heartbeatInterval inside jstp package starting with v1.0.0, but we forgot to update the impress configs respectively.

tshemsedinov commented 5 years ago

Fixed in 0.7.2 see: https://github.com/metarhia/impress/pull/899