Improve sync_ch performance in Django debug mode #265
Conversation
Prevents memory exhaustion in Django debug mode
Also remove remove log_and_ignore_exceptions()
Codecov Report
@@ Coverage Diff @@
## develop #265 +/- ##
===========================================
+ Coverage 93.39% 93.64% +0.25%
===========================================
Files 56 56
Lines 2044 2047 +3
Branches 189 190 +1
===========================================
+ Hits 1909 1917 +8
+ Misses 114 109 -5
Partials 21 21
Continue to review full report at Codecov.
|
| class Command(BaseCommand): | ||
| """Companies House sync command.""" | ||
|
|
||
| def handle(self, *args, **options): | ||
| """Handle.""" | ||
| try: | ||
| sync_ch(tmp_file_creator=tempfile.TemporaryFile, truncate_first=True) | ||
| except Exception as e: | ||
| with log_and_ignore_exceptions(): |
There was a problem hiding this comment.
Why remove that? If sentry is down (and this happened a couple of times), the whole process will blow up without this.
There was a problem hiding this comment.
So what? It's already blown up by this point and the exception chain will be there.
The old code was broken because self.stderr.write(e) takes a string and not an exception. Even if you fixed it, it would mess up the exit code because it was swallowing up any errors.
|
The explicit capturing of CLI command exceptions for Sentry is no longer needed, because they fixed the bug stopping that happening automatically in Raven 6.1. |
In debug mode, django keeps a history of all SQL statements executed. This was causing the sync_ch command to exhaust the available memory. The main change here is to call
django.db.reset_queries()to clear that history after each batch is inserted in the sync_ch command.There's also more logging added to give more visibility of what the command is doing, and changes to the local logging config so that standard Python logging works more seamlessly.