-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve the efficiency of DbLog
migration for Django
#2949
Improve the efficiency of DbLog
migration for Django
#2949
Conversation
I tested this on the mat2D database and the |
The old implementation was using Django's ORM to get the count and rows of certain rows in the `DbLog` table. However, this was generating extremely inefficient queries that made the migration run for days on big databases. A similar problem was encountered with the original SqlAlchemy implementation. That was solved in an earlier commit `80a0aa9117f399a5385a30ba0ce3e2afc7016a4e`, which created a more efficient query. We now replace the Django ORM solution with the same custom SQL query as used for SqlAlchemy. This should significantly improve the efficiency of this migration.
8ca4f75
to
01b2eba
Compare
DbLog
migration for Django DbLog
migration for Django
@sphuber just double checking that you also verified that the DbLogs are still there and correct in the mat2D, before merging |
I did, but it is hard to say. The unit tests should cover this. I forgot to note the number of rows in the original database for the |
I think it's ok then (maybe just run a few verdi commands to get logs and check the they work). I agree that tests should ideally already cover this |
2 similar comments
Thanks Seb! I also believe that if the tests pass and you did a basic check that everything is as it should, it should be OK |
Fixes #2539
The old implementation was using Django's ORM to get the count and rows
of certain rows in the
DbLog
table. However, this was generatingextremely inefficient queries that made the migration run for days on
big databases. A similar problem was encountered with the original
SqlAlchemy implementation. That was solved in an earlier commit
80a0aa9117f399a5385a30ba0ce3e2afc7016a4e
, which created a moreefficient query. We now replace the Django ORM solution with the same
custom SQL query as used for SqlAlchemy. This should significantly
improve the efficiency of this migration.