davidyaha / graphql-redis-subscriptions

A graphql subscriptions implementation using redis and apollo's graphql-subscriptions
MIT License
1.11k stars 125 forks source link

Add an optional filterFn to PubSubAsyncIterator to workaround performance issues found in withFilter #601

Open davisg123 opened 1 year ago

davisg123 commented 1 year ago

Our production application filters a high number of messages, more than 99% are filtered out. We suffered an issue of unbounded memory growth caused by the behavior of withFilter. Our understanding is that withFilter will continue chaining promises together until a message is accepted. Because of our aggressive filtering our containers would quickly run out of memory

Screenshot%202023-05-01%20at%2011 43 15%20AM

We have since improved our sharding behavior so that we are filtering less messages, but we still think there is an improvement that can be made to filtering.

This PR adds a filterFn to PubSubAsyncIterator so that messages can be filtered immediately before insertion into the queue. We believe this is a performance win regardless of how many messages are being filtered. Thank you!

davidyaha commented 4 months ago

Hey @davisg123! Thanks for the PR and sorry for my late review.

I look at the example provided and I can't help but wondering why the event is even published in the first place? This looks like not the responsibility of something like asnycIterator on it's own. The whole point is to allow chaining of filters so that there is a "stream-like" behaviour rather than a simple "one filter rules all" approach.

Could you perhaps suggest a more complex example where this filter function logic cannot be applied before calling the publish method?