Skip to content

Conversation

@rodrigok
Copy link
Member

@rodrigok rodrigok commented Jun 12, 2021

Proposed changes (including videos or screenshots)

Stream Cast uses a different approach to broadcast data to the instances, it uses the DDP subscription method that requires a collection on the other side, if no collection exists with the given name broadcast-stream it caches in memory waiting for the collection to be set later. The cache is cleared only when a reconnection happens.

This PR overrides the function that processes the data for that specific connection, preventing the cache and everything else to be processed since we already have our low-level listener to process the data.

More info

Bellow an example of the memory usage of a deployment using the normal matrix event broadcast among instances during the first 9 days and then using the Stream Cast to broadcast events during the last 5 days (with a restart every day due to the memory usage).
image

Stream Cast uses a different approach to broadcast data to the instances, it uses the DDP subscription method that requireds a collecction on the other side, if no collection exists with the given name `broadcast-stream` it caches in memory waiting for the collection to be set later. The cache is cleared only when a reconnection happens.

This PR overrides the function that process the data for that specific connection, preventing the cache and everything else to be processed since we already have our low level listener to process the data.
@rodrigok rodrigok requested review from ggazzo and sampaiodiego June 12, 2021 14:27
// Ignore collection events to prevent store in memory
// necessary to prevent memory lead due to the usage
// of DDP subscribe
connection._livedata_data = () => {};
Copy link
Member

Choose a reason for hiding this comment

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

doing this actually breaks streamcast, all events are ignored and communication doesn't happen.

another way of preventing this leak to happen, is to actually register a store for the broadcast-stream collection.

so I changed the current line 180 (connection._stream.on('message', function(raw_msg) { for the following code:

	connection.registerStore('broadcast-stream', {
		update({ fields }) {
			const { streamName, eventName, args } = fields;

			if (!streamName || !eventName || !args) {
				return;
			}

			if (connection.broadcastAuth !== true) {
				return 'not-authorized';
			}

			const instance = StreamerCentral.instances[streamName];
			if (!instance) {
				return 'stream-not-exists';
			}

			if (instance.serverOnly) {
				return instance.__emit(eventName, ...args);
			}
			return instance._emit(eventName, args);
		},
	});

looks like even a less hacky solution, wdyt?

@sampaiodiego sampaiodiego merged commit be0e93a into develop Jun 21, 2021
@sampaiodiego sampaiodiego deleted the fix-streamcast-memory-leak branch June 21, 2021 03:57
@sampaiodiego sampaiodiego mentioned this pull request Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants