--
Ticket URL: <https://code.djangoproject.com/ticket/34865>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* Attachment "databasewrapper.png" added.
* status: new => closed
* resolution: => duplicate
Comment:
Fabio, thanks for the report. Is there a reason to open a new ticket and
don't add your findings in the #33497? As far as I'm aware, this is a
duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:1>
Comment (by Fábio Domingues):
Replying to [comment:1 Mariusz Felisiak]:
> Fabio, thanks for the report. Is there a reason to open a new ticket and
don't add your findings in the #33497? As far as I'm aware, this is a
duplicate.
Fixing this memory leak will not solve the problem of the persistent
connections not been reuse, only the fact that they will not pill up (and
starve the DB) if inaccessible.
But I'm new here, still learning. Should I move the findings to #33497?
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:2>
* cc: Carlton Gibson, Andrew Godwin, Andreas Pelme (added)
* status: closed => new
* resolution: duplicate =>
* stage: Unreviewed => Accepted
Comment:
We can keep this separately.
Tentatively accepted, I'd like to check out the patch proposal.
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:3>
Comment (by Carlton Gibson):
> ...I'd like to check out the patch proposal.
Yes, please.
Any chance you could give a minimal example showing the behaviour here, to
look at?
Various folks have reported ''leaky'' behaviour on Channels/Daphne, but
we've not been able to pin it down.
> … ends up in a circular reference ... and the GC could not cleanup the
objects.
Just out of interest, could you try throwing in a `gc.collect()` — that
**should** get it (but `WeakRef` is hopefully right in this case.)
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:4>
Comment (by Fábio Domingues):
Tested using Uvicorn and Daphne, the database engine should not matter, I
tested with PostgreSQL, `CONN_MAX_AGE` could be any value and the effect
is the same using ''async'' or ''sync'' views.
{{{#!python
get_refs = lambda: [o for o in gc.get_objects() if isinstance(o,
BaseDatabaseWrapper)]
plot_refs = lambda: objgraph.show_backrefs(get_refs(), filename="sample-
graph.png")
def sync_view(request):
MyModel.objects.count()
# gc.collect()
# plot_refs()
return HttpResponse(f"Number of BaseDatabaseWrapper instances:
{len(get_refs())}")
async def async_view(request):
await MyModel.objects.acount()
# gc.collect()
# plot_refs()
return HttpResponse(f"Number of BaseDatabaseWrapper instances:
{len(get_refs())}")
}}}
On each refresh you should see the number of instances increase. If you
plot the references you would see many diagrams like the one in the
attachment. Throwing `gc.collect()` actually get it, but it take some
milliseconds, even if the underlying database connection is already
closed.
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:5>
* Attachment "fix_34865.patch" added.
Possible approach
--
Ticket URL: <https://code.djangoproject.com/ticket/34865>
Comment (by Fábio Domingues):
One more detail. Using `objgraph.show_growth` without doing `gc.collect`
give us this on each new request:
{{{
list 1990 +10
dict 6798 +3
deque 20 +2
lock 43 +2
OrderedDict 11 +2
partial 24 +2
ReferenceType 3848 +1
DatabaseWrapper 6 +1
DatabaseClient 6 +1
DatabaseCreation 6 +1
DatabaseFeatures 6 +1
DatabaseIntrospection 6 +1
DatabaseOperations 6 +1
BaseDatabaseValidation 6 +1
DatabaseErrorWrapper 5 +1
PGconn 5 +1
Connection 5 +1
PrepareManager 5 +1
AdaptersMap 7 +1
TypesRegistry 7 +1
}}}
with the previous patch the output of `show_growth` is empty.
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:6>
* Attachment "fix_34865.patch" removed.
Possible approach
--
Ticket URL: <https://code.djangoproject.com/ticket/34865>
* Attachment "fix_34865.patch" added.
Possible approach.
* Attachment "fix_34865.patch" removed.
* cc: David Wobrock (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:7>
* owner: nobody => Priyank Panchal
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:8>
Comment (by Florian Apolloner):
> Throwing gc.collect() actually get it, but it take some milliseconds,
even if the underlying database connection is already closed.
with "actually get it" do you mean that it cleans it up properly? If yes
there does not seem to be real leak here since garbage collection usually
runs every now and then (or do I miss something). It might still be
beneficial to improve the performance here but that might require a test
or two :D
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:9>
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:10>
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Marking as needs improvement per Florian's comment.
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:11>
* cc: Florian Apolloner (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34865#comment:12>