vapor / redis

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

beta.2 #156

Closed jordanebelanger closed 4 years ago

jordanebelanger commented 4 years ago

@tanner0101 I'm also thinking, since we're using a pool, we may want to expose the pool directly. There are certain cases where you may want to send more than one command one after an other, in those cases, doing 2 redis.send in a row will thread lock twice for no reason since you could just be holding the connection for the duration of the 2+ calls instead.

It would just be app.redis.pool

tanner0101 commented 4 years ago

doing 2 redis.send in a row will thread lock twice for no reason since you could just be holding the connection for the duration of the 2+ calls instead.

Hmm why would that thread lock at all?

jordanebelanger commented 4 years ago

doing 2 redis.send in a row will thread lock twice for no reason since you could just be holding the connection for the duration of the 2+ calls instead.

Hmm why would that thread lock at all?

Unless I am mistaken, the older ConnectionPool used to synchronize access on "requestConnection" calls with a lock. So if that was indeed the case, 2 or more redis send calls in a row would needlessly lock the pool multiple times to retrieve a connection each time.

But it looks like the new EventLoopConnectionPool doesn't need to synchronize anymore upon a requestConnection call so indeed you are right no lock is needed.