[PULP-1777] Fix postgres 65k param limit on the Copy API - #1506
Conversation
c0d42c8 to
55ffb0c
Compare
|
Look at this comment to see some samples of how that new test machinery works: pulp/pulp_rpm#4538 (comment) I plan to extract those test utilities somehwere, maybe pulpcore, but IHO it's not a big deal to duplicate it here. |
|
There was a test hanging, I'll investigate it tomorrow. |
55ffb0c to
690e8ca
Compare
|
SQL Diff (filtered by call made from within And a nice read about unnest/arrays (although it talks about inserts, it explains well how the idea behind it in general): |
690e8ca to
d7135f5
Compare
* Add growth rate based tests * Fix linear SQL params count growth rate in copy API * Add n+1 detector test for copy API and fixes for some of them Assisted-By: Claude Sonnet 5
d7135f5 to
4e28ab5
Compare
| IgnoreFromPath( | ||
| pattern=r"pulp_deb/app/models/repository\.py:\d+ in handle_duplicate_releases", | ||
| reason="needs further investigation on real impact", | ||
| ), |
There was a problem hiding this comment.
I've skipped dealing with this to avoid doing too many real plugin changes in one PR.
If you remove this ignore item and run the unit test you'll see exactly what ORM call is causing a N+1 and you can view the SQL report with the snippet (the unit test error output should be informative enough).
oci-env exec cat /tmp/pytest-artifacts/test_query_count_is_size_invariant.packages_across_distributions.small.sql
I had a quick look and the cause for this N+1 is that the code queries Content filtering by releases, and inside a loop you call release.distribution, which triggers a new Release query call per item to fetch the corresponding distribution.
This uses the same approach used in pulp_rpm with query param growth-rate based tests. It's an alternative approach to #1499
Basically, we record every query within the unit test using django utilities, do some data treatment, hook in python's inspect module and we get traceable code (in points where querysets are evaluated) to generated SQL, plus some sql metadata. And among that metadata is query params count.
The test uilities themselves can be improved, but it already catches the bulk of scenarios which can potentially trigger this param limit error. Plus an experimental n+1 detector 👀 (which you didn't have in copy! apparently at least, congratz)
Assisted-By: Claude Sonnet 5
📜 Checklist
See: Pull Request Walkthrough