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

Optimize complex GraphQL queries #171

Closed
dgarros opened this issue Mar 18, 2021 · 9 comments · Fixed by #769
Closed

Optimize complex GraphQL queries #171

dgarros opened this issue Mar 18, 2021 · 9 comments · Fixed by #769
Assignees
Labels
type: feature Introduction of new or enhanced functionality to the application
Milestone

Comments

@dgarros
Copy link
Contributor

dgarros commented Mar 18, 2021

Environment

  • Python version: 3.7
  • Nautobot version: 1.0.0b2

Proposed Functionality

Leverage the ORM capabilities to query multiple results at once with prefetch_related and select_related to optimize some complex GraphQL queries and improve their response time

Use Case

A query like this one currently takes 1.87 sec (with caching) to return in the sandbox environment because we are querying the site and the region for each device.

query {
  devices {
    name
    site {
      name
      region {
        name
      }
    }
  }
}

With some optimization we should be able to return this query much faster

Database Changes

No

External Dependencies

Not sure yet but we should investigate some existing packages like graphene-django-optimizer that are trying to solve the same problem.

@jathanism jathanism added the type: feature Introduction of new or enhanced functionality to the application label Mar 18, 2021
@jathanism
Copy link
Contributor

Do you feel this is this something we need try to tackle for v1.0.0 or can it wait til after?

@dgarros
Copy link
Contributor Author

dgarros commented Mar 18, 2021

Not mandatory for 1.0.0 but would be good to have it for 1.1.0 if possible
I'll be happy to help with this one as time permit

@jathanism
Copy link
Contributor

Not mandatory for 1.0.0 but would be good to have it for 1.1.0 if possible

I'll be happy to help with this one as time permit

Great thanks. Just helps me prioritize the final stretch to 1.0.0-final.

@jathanism
Copy link
Contributor

jathanism commented May 28, 2021

A few parts to this:

  • Prototype and validate that graphene-django-optimize could be a good starting point
  • Implement the required changes to optimize queries
  • Perform performance testing to assert that queries are indeed optimized

@carbonarok
Copy link
Contributor

carbonarok commented Aug 20, 2021

We are in a bit of a tight spot with this issue and module. Because of the AssignedObjectType graphql type, we are stuggling to attach the optermiser to nautobot. It doesnt seem like the optermiser module supports graphene.Union classes. Can be seen in #769.

The only way I might have found to get around this is by using @gql_optimizer.resolver_hints, where you can define a callback function for a prefetched or select related attribute. I am currently having issues getting this working because of the poor documentation round this feature.

This leads onto the next issue with this module, the only documentation can be found here in the README. There doesnt seem to be much support for the module, in one of the open issues you can see the owner trying to give away the project to another organisation.

I think maybe if a few of us worked together to get a prototype working we might be able to, but my worry is, would it be worth it with the limited support once it works. One possibility would be taking over the module or forking and supporting it ourselves but do we want to take on that work?

@jedelman8
Copy link
Contributor

Thanks @carbonarok . We are going to close this for now. Once we have more feedback from GraphQL power users and see if there are issues, we can re-evaluate the time needed to get this going and implemented.

@dgarros
Copy link
Contributor Author

dgarros commented Aug 27, 2021

@carbonarok after our last discussion I was under the impression that your branch should work so I did some testing on it and as far as I can tell it's working pretty well :)

I wrote a management command to do some testing and I found some interesting results.

For my testing I used a dataset similar to the sandbox and I created 4 GraphQL queries that would present different use cases. You can see the 4 queries that I used here, I named them to simplify the reporting.

For each query, I measured how many SQL requests were generated, the response time with cache and without cache.

Number of SQL queries generated per GraphQL query

Just by looking at the number of SQL queries generated per GraphQL query, we can see a huge optimization, in some cases a reduction of the number of queries by a factor of 100x or more

Number of SQL Queries per GraphQL query next carbonarok:kc_graphene_optimizer Reduction Factor
devices_interfaces_ips_edge 2523 5 504
devices_interfaces_all 210 3 70
devices_sites_regions_all 232 2 116
golden_config_device_extensive 261 124 2.1

Looking at the response time, we can also see a significant improvement with carbonarok:kc_graphene_optimizer branch

Response time per GraphQL query with and without cache, without optimization

branch next

┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃                          Query ┃ No Cache (avg)            ┃ Cache (avg)              ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━┩
│    devices_interfaces_ips_edge │ 9.0797 / 9.6646 / 10.4981 │ 3.4109 / 3.6083 / 3.7353 │
│         devices_interfaces_all │ 1.4679 / 1.6171 / 1.7387  │ 0.6659 / 0.7834 / 0.8886 │
│      devices_sites_regions_all │ 0.6331 / 0.7080 / 0.7825  │ 0.5518 / 0.5884 / 0.6143 │
│ golden_config_device_extensive │ 1.4284 / 1.5762 / 1.6441  │ 0.4353 / 0.4524 / 0.4656 │
└────────────────────────────────┴───────────────────────────┴──────────────────────────┘

Response time per GraphQL query with and without cache, WITH optimization

branch carbonarok:kc_graphene_optimizer

┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃                          Query ┃ No Cache (min / avg / max) ┃ Cache (min / avg / max)  ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━┩
│    devices_interfaces_ips_edge │ 1.7424 / 1.7927 / 1.8460   │ 1.5414 / 1.5930 / 1.7110 │ 
│         devices_interfaces_all │ 0.8124 / 0.8490 / 0.9006   │ 0.4993 / 0.5887 / 0.6286 │
│      devices_sites_regions_all │ 0.1252 / 0.1501 / 0.2349   │ 0.0860 / 0.0909 / 0.1007 │ 
│ golden_config_device_extensive │ 1.2699 / 1.3087 / 1.4185   │ 0.3202 / 0.3294 / 0.3407 │ 
└────────────────────────────────┴────────────────────────────┴──────────────────────────┘

I think this is pretty good, Great work @carbonarok :)
We might not be able to optimize all queries yet but that would be great to get these optimizations

@jathanism
Copy link
Contributor

This is I N C R E D I B L E!! The query optimization on devices_interfaces_ips_edge alone is jaw-dropping. Great work @carbonarok. And we couldn't have done this without you @dgarros, so bravo to you as well!

@glennmatthews
Copy link
Contributor

Needs someone to pick up #769 and see it through to completion.

jathanism added a commit that referenced this issue Oct 27, 2021
@jathanism jathanism self-assigned this Oct 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature Introduction of new or enhanced functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants