john-bodley opened a new pull request, #23113: URL: https://github.com/apache/superset/pull/23113
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY https://github.com/apache/superset/pull/22413 introduced logic for eager loading of the columns/metrics (as well as their back references) for a dataset, however the eagerness comes at a cost, i.e., the RESTful API GET `/api/v1/explore` endpoint is less performant given that all when fetching a slice or dataset the columns/metrics relationships are eagerly loaded. This PR provides a tradeoff where we lazily load the columns/metrics relationships which negatively impacts the `/api/v1/dataset` endpoint but speeds up the `/api/v1/explore` endpoint (the later is used significantly more than the former). The TL;DR is the proposed behavior is more akin to how objects were lazy loaded prior to https://github.com/apache/superset/pull/22413 but there is significant performance improvements by not leveraging the `joined` [relationship](https://docs.sqlalchemy.org/en/14/orm/relationship_api.html#sqlalchemy.orm.relationship). Specifically for Airbnb's uber dataset the results are: | Endpoint | Current | Proposed | | ---------------- | -------- | --------- | | `/api/v1/dataset` | ~ 7 s | ~ 26 s† | | `/api/v1/explore` | ~ 16 s | ~ 5 s | † Note even though this is slower it is significantly faster than prior to https://github.com/apache/superset/pull/22413 which used to time out. Furthermore a significantly cost is in the (now) lazy loading of the back references, i.e., `metric.table` which likely could be refactored in code. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS CI and local performance testing. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
