vapor / redis

Vapor provider for RediStack
MIT License
459 stars 58 forks source link

Connection pooling interfering with PubSub #194

Open wilg opened 2 years ago

wilg commented 2 years ago

I'm trying to debug an issue with my Redis PubSub where it's throwing a warning about "(RediStack) connection attempted to create a PubSub subscription".

I think what may be happening is that the PubSub code below pulls a connection out of the connection pool, but does not remove it from the pool. Meaning eventually that connection that is performing PubSub will get vended back out to some consumer, and because it has been placed in PubSub mode, will no longer be able to send.

https://github.com/vapor/redis/blob/50d6cea0a96558baa0360e435ea02f5ddf8803c4/Sources/Redis/Application.Redis%2BPubSub.swift#L8-L27

Mordil commented 2 years ago

Do you have a snippet of your code that runs into this?

wilg commented 2 years ago

I don't have a good snippet unfortunately, but the issue seemed to occur when I had subscribed to a channel and then did a large number of sends such that it exhausted the pool.

I was also able to work around it by doing my subscriptions on a RediStack connection that I initialized separately. So I think that is consistent with this theory?

Can try to put something together if this isn't making sense.

Mordil commented 2 years ago

I’m curious to find if it’s an issue with RediStack’s PubSub implementation through the connection pool itself, or Vapor’s support of it

wilg commented 2 years ago

I'm not sure – PubSub essentially "poisons" a connection so it can no longer be used to publish, so in theory it shouldn't be returned to the pool at all (or maybe if it's done subscribing).

The reason I posted the issue here and mentioned the code above is it looks like this repo has special handling of the PubSub connection (it gets it from the pool and always uses that one for PubSub) but it doesn't remove it from the pool itself (as far as I can tell).

Mordil commented 2 years ago

Yes, I'm intimately aware of this. My comment was me thinking out loud to find the issue. I'm strongly suspecting that it's Vapor in the wrong here, I'll dig deeper as soon as I can

wilg commented 2 years ago

Cool, just clarifying! It was very late when I posted this issue so I'm also trying to remember what I was thinking last night.