HashClient does not appear to be thread-safe about pymemcache HOT 6 CLOSED

I can't comment on the thread-safety of HashClient, as it has been awhile since I looked at the code (and I have never used Python threads), but I concur that the intention for PooledClient is as @jaysoffian says: PooledClient should manage instances of HashClient, and not vice-versa. In this case my first inclination would be to create a HashPooledClient similar to how PooledClient works. If possible, it would be nice to share some code between those classes.

from pymemcache.

After some more reading, I'm dubious about the design of HashClient. The memcached docs recommend:

In the dark days of memcached, the default was to always "failover", by trying the next server in the list. That way if a server crashes, its keys will get reassigned to other instances and everything moves on happily. ... Modern life encourages the use of "Failure", when possible. That is, if the server you intend to fetch or store a cache entry to is unavailable, simply proceed as though it was a cache miss.

In my case, I've decided to not bother with HashClient. I'm just going to create multiple instances of PooledClient which are chosen using RendezvousHash and not bother with retry logic, adding/removing servers, etc. KISS and all that. I'll revisit this issue when I'm done with that. Minimally, I'll submit a PR updating the docs about thread-safety and this library.

In any case, thanks you for this library I chose it after reading over the code and the base Client is solid.

from pymemcache.

Can this be considered closed as of v3.2.0?

from pymemcache.

@abhijit-15 is there a specific change that was released that you're thinking of?

from pymemcache.

@jparise Yes. So as of v3.0.0, use_pooling has been made available and HashClient sends encoding to its pooled clients. IMO even if there are still any issues, the ability to handle custom client classes (for Hash & PooledClient as of v3.2.0) should be sufficient for user to deal with this?

from pymemcache.

That sounds good. Thanks for leaving the detailed comment so others who find this issue will have that additional context.

from pymemcache.