Re: [Django] #34746: High CPU/memory consumption when a 5XX is raised with large local variables
Django
--------------------------------------+------------------------------------
Reporter: Rémi Dupré | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Natalia Bidart):
* type: Bug => Cleanup/optimization
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I have managed to reproduce the issue, thank you for the example. Indeed
if `v = pprint(v)` is replaced by something that does not render the whole
variable, the memory and CPU usage do not spike.
Accepting so we can evaluate whether an alternative could be used instead
of `pprint` that would account for the size limit before printing.
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: Rémi Dupré | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Natalia Bidart):
After some experimentation following the suggested link to reprlib,
perhaps something like this could be a good alternative? (it needs better
code formatting, more reprlib limits sets and tests, of course)
{{{
#!diff
--- a/django/views/debug.py
+++ b/django/views/debug.py
@@ -347,12 +347,17 @@ class ExceptionReporter:
self.template_does_not_exist = True
self.postmortem = self.exc_value.chain or [self.exc_value]
+ import reprlib
+ repr_instance = reprlib.Repr()
+ repr_instance.maxstring = 5000
+ repr_instance.maxlist = 1000
+
frames = self.get_traceback_frames()
for i, frame in enumerate(frames):
if "vars" in frame:
frame_vars = []
for k, v in frame["vars"]:
- v = pprint(v)
+ v = repr_instance.repr(v)
# Trim large blobs of data
if len(v) > 4096:
v = "%s… <trimmed %d bytes string>" % (v[0:4096],
len(v))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:3>
Django
Reporter: Rémi Dupré | Owner: Vishesh
Type: | Garg
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Vishesh Garg):
* owner: (none) => Vishesh Garg
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:4>
Django
Reporter: Rémi Dupré | Owner: (none)
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Vishesh Garg):
* owner: Vishesh Garg => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:5>
Django
Reporter: Rémi Dupré | Owner: Vishesh
Type: | Garg
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Vishesh Garg):
* owner: (none) => Vishesh Garg
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Vishesh
Type: | Garg
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/17127 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Vishesh
Type: | Garg
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):
I think using reprlib is acceptable, this is exactly the kind of situation
it’s designed for. It will be a shame to lose the pretty-printing aspect,
but perhaps reprlib/pprint will grow appropriate functionality in the
future.
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:8>
Django
-------------------------------------+-------------------------------------
Type: | Kumar Verma
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Vishesh Garg => Yash Kumar Verma
Comment:
Hello folks, I saw the two pull requests that were attached in the ticket
are stale, and the original author have both closed. I'm interesting in
picking this as my first ticket, can I proceed?
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Yash
Type: | Kumar Verma
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Yash Kumar Verma):
I was:
* able to verify the following
* when exception is raised after declaring a large variable, memory
usage shoots up.
* when exception is not raised after declaring a large variable,
memory remains in live with normal operations.
'''Setup I used to reproduce the error and suggested solution'''
* added a middleware which logs the change in memory after each request.
* created two views, one which throws error and one that does not.
* observed the memory usage in both the cases.
'''Current'''
[[Image(https://github.com/YashKumarVerma/django-
pocs/blob/master/poc01/poc01/2023-12-03-14-18-23.png?raw=true)]]
'''After using reprlib'''
[[Image(https://github.com/YashKumarVerma/django-
pocs/blob/master/poc01/poc01/2023-12-03-14-35-23.png?raw=true)]]
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner:
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Yash Kumar Verma => keerthivasansa
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:11>
Django
-------------------------------------+-------------------------------------
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Ready for checkin => Accepted
Comment:
You should not mark your own PRs as "ready for checkin".
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Ah sorry, I wanted to set this ticket as ready to be reviewed as mentioned
by a contributor in the PR. How do I do that?
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Replying to [comment:14 Keerthi Vasan S A]:
> Ah sorry, I wanted to set this ticket as ready to be reviewed as
mentioned by a contributor in the PR. How do I do that?
Hi!
The proper procedure to get a re-review is described in
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/#patch-style the PR review checklist]. Basically you
need to unset the relevant flags in this ticket so the PR gets added back
to the "patches needing review" section of the
[https://dashboard.djangoproject.com/ Django Developer Dahsboard].
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Gotcha, thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Is this still being worked on or should I handle t?
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Replying to [comment:19 Ahmed Ibrahim]:
the PR, this is still being work on and I'll provide more feedback in the
PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: Rémi Dupré | Owner: Keerthi
Type: | Vasan S A
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Hi,
Is this ticket open to be taken over?
--
Ticket URL: <https://code.djangoproject.com/ticket/34746#comment:21>