Opened 3 years ago

Last modified 3 months ago

#33625 assigned Cleanup/optimization

Under ASGI, PyLibMCCache closes memcached connection after every request

Reported by: Anders Kaseorg Owned by: Pablo Montepagano
Component: Core (Cache system) Version: 4.0
Severity: Normal Keywords: asgi, async
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Anders Kaseorg)

When I run an application that uses django.core.cache.backends.memcached.PyLibMCCache with WSGI under Gunicorn, it makes a single connection to memcached and leaves it open, as I expect.

But when I run it with ASGI under uvicorn, it makes a different connection to memcached for every request and immediately closes it. That is, #11331/#15324 has returned. This at least adds undesirable latency, and may also eventually lead to connection failures as described in #11331.

I think this happens because e3f6e18513224c8ad081e5a19da641f49b0b43da switched the thread-local cache objects to task-local cache objects. That was justified in #31253 by “a potential race condition if two coroutines touch the same cache object at exactly the same time”. But that justification doesn’t make sense: two coroutines cannot be running the same synchronous code on the same thread at the same time, so thread-local objects were already safe.

Now that asynchronous cache backend support is being developed, the argument is slightly different—but it seems obvious to expect that any asynchronous cache backend should at least properly support concurrent use from multiple asynchronous tasks on the same thread, so thread-local objects should still be safe.

Change History (9)

comment:1 by Anders Kaseorg, 3 years ago

Possibly related: #33497 (for database connections).

comment:2 by Carlton Gibson, 3 years ago

Keywords: asgi async added
Triage Stage: UnreviewedAccepted

I didn't test, but seems reasonable. Thanks for the report Anders.

(If you could attach a reproducing project with exact steps, that would save a cycle later.)

comment:3 by Anders Kaseorg, 3 years ago

Sure. Here’s a minimal test project. Clone it, run memcached -vv along with gunicorn mctest_wsgi or uvicorn mctest_asgi:application, visit http://localhost:8000 several times, and check whether memcached -vv is repeating lines like “new auto-negotiating client connection” and “connection closed.”

Last edited 3 years ago by Anders Kaseorg (previous) (diff)

comment:4 by Carlton Gibson, 3 years ago

Super, thanks. 🏅

comment:5 by Pablo Montepagano, 2 years ago

Owner: changed from nobody to Pablo Montepagano
Status: newassigned

I'd like to try to solve this ticket. If I understand correctly, what would be needed is a connection pool for PyLibMCCache and for PyMemcacheCache that works well under ASGI. I'm thinking of doing something similar to what was done in this repo: https://github.com/Andrew-Chen-Wang/django-async-redis

Before I start developing, do we have a good way to do integration testing for this? I'll try to look into that first.

in reply to:  5 comment:6 by Carlton Gibson, 2 years ago

Replying to Pablo Montepagano:

Before I start developing, do we have a good way to do integration testing for this? I'll try to look into that first.

Good question! We have a few ASGI related tickets for which this is on the agenda. Currently working on putting test projects together and then using Locust to run requests against it. If you fancied going that route I'd be happy to input. (Perhaps ping me on a sample repo... or...) Thanks!

comment:7 by Anders Kaseorg, 2 years ago

Description: modified (diff)

We don’t need a new pool. Both pylibmc and pymemcache already have one.

It seems to me it’d be best to leave pooling up to the backend library, because it’s in a better position to know what kinds of concurrency are safe.

comment:8 by Natalia Bidart, 3 months ago

I believe that #35757 is a dupe of this one.

comment:9 by Harm Verhagen, 3 months ago

NB: #35757 is about breaking runserver (stops working), this one (#33625) is a performance optimization.

Note: See TracTickets for help on using tickets.
Back to Top