-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
API optimizations for tagged objects #2438
Comments
Do you have custom fields for your devices? Afaik each custom field generates another query for each device into the custom fields table. This is why the usage of custom fields should be limited. |
I think @Anthony25 can answer that but I don't think we got custom fields. Also with the fix provided ( #2437 ) we reduced the query load with the same data. |
/offtopic: hi @sieben , could you provide some help to get this 'debug toolbar' to show up? I'm assuming that the |
We do have custom fields, but it's the tags that are not prefetched which is causing the issue. As the default behavior of django is to fetch the foreign objects JIT, it has to do 1 query per item to select the associated tags. We managed to fix that for the devices in our PR, but it has to be done for every model that uses tags. Edit: FYI, we managed to decrease the number of queries from ~1000 to 10, with 1000 devices listed from the API |
Have you done any benchmarking before and after the change? For example, populate 1000 devices with 10 tags each. I'm curious what the overall response time is before and after the change to |
I benchmarked on the API call Without the 'tags' being prefetched in the Fixing the
File patched to gain these improvements: https://github.com/digitalocean/netbox/blob/bcf22831e2cc8a0c3021f0a73afd0ddc4a1b6b8d/netbox/ipam/api/views.py#L248 |
@sdktr: thanks a lot for the benchmark! |
Environment
Steps to Reproduce
Expected Behavior
I expected the number of SQL queries to not grow as the number of devices grows
Observed Behavior
The amount of SQL queries increased
#2437
The text was updated successfully, but these errors were encountered: