Skip to content
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

Use raw sql to execute deletes #2953

Merged
merged 10 commits into from
Jun 17, 2021
Merged

Use raw sql to execute deletes #2953

merged 10 commits into from
Jun 17, 2021

Conversation

Red-HAP
Copy link
Contributor

@Red-HAP Red-HAP commented Jun 11, 2021

Jira Ticket

COST-1490

Description

Use raw sql vs ORM to do deletes. This change takes the ORM query and uses django's own SQLDeleteCompiler to get the text of the SQL statement plus the parameters. This is then processed directly via connection and cursor objects without using django ORM to attempt to verify and handle the foreign keys.

Provider().delete() is now tuned up as well to call a func named cascade_delete that will walk ORM relations and execute SQL to delete related records bottom-up.

Testing

The resulting delete action should be no different from usual. But the memory usage should not expand to OOM state during delete operations for expired data or anything else that a db_cleaner would be called for.

If you run make docker-reinitdb-with-sources then run a django shell, you can watch the cascade delete work.
How-To-Do-It Lessons:

  1. Navigate to top-of-repo
  2. make docker-reinitdb-with-sources
  3. make load-test-customer-data
  4. You can check the relations from Provider to the line-item tables, if you want.
  5. cd koku
  6. export KOKU_LOG_LEVEL=DEBUG
  7. python ./manage.py shell
  8. from api.provider.models import Provider
  9. p = Provider.objects.first() (or p = Provider.objects.get(pk=<UUID>) if you have identified a provider to use.)
  10. p.delete()
  11. You should see a flurry of log messages with Level and Cascade in them. With logging set to debug, you'll also see the formatted SQL statements that are being executed.
  12. ctrl+d to exit the shell
  13. ``unset KOKU_LOG_LEVEL` to clear debug logging

@Red-HAP Red-HAP added the API Django/Python/Config label Jun 11, 2021
@Red-HAP Red-HAP requested a review from a team June 11, 2021 19:23
@Red-HAP Red-HAP self-assigned this Jun 11, 2021
@codecov
Copy link

codecov bot commented Jun 11, 2021

Codecov Report

Merging #2953 (adff7bf) into master (d8dc13c) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #2953    +/-   ##
=======================================
  Coverage    95.0%   95.1%            
=======================================
  Files         297     298     +1     
  Lines       23647   23759   +112     
  Branches     2632    2632            
=======================================
+ Hits        22475   22585   +110     
- Misses        703     704     +1     
- Partials      469     470     +1     

@dccurtis
Copy link
Contributor

@Red-HAP Can this be extended to cover the flow for removing a provider here:

self.model.delete()

This is what is called when a source is destroyed and is the path in which we are suspecting the Django ORM being inefficient at managing the foreign key relationships while deleting.

Copy link
Contributor

@dccurtis dccurtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work HAP!

@Red-HAP Red-HAP merged commit e414b94 into master Jun 17, 2021
@Red-HAP Red-HAP deleted the COST-1490-provider-delete branch June 17, 2021 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Django/Python/Config
Projects
None yet
2 participants