etr2460 opened a new pull request #7752: [SQL Lab] Reduce db load on /queries endpoint URL: https://github.com/apache/incubator-superset/pull/7752 ### CATEGORY Choose one - [x] Bug Fix - [ ] Enhancement (new features, refinement) - [ ] Refactor - [ ] Add tests - [ ] Build / Development Environment - [ ] Documentation ### SUMMARY <!--- Describe the change below, including rationale and design decisions --> For the past few months, we were seeing errors like this during times of heavy load for superset: ``` sqlalchemy.exc:TimeoutError: QueuePool limit of size 30 overflow 15 reached, connection timed out, timeout 30 (Background on this error at: http://sqlalche.me/e/3o7r) ``` When digging further, the stack trace revealed that these errors occurred while evaluating the `has_access` and `has_access_api` decorators. These decorators (which were added to every view in https://github.com/apache/incubator-superset/pull/6553) make many selects against superset's database resulting in an extra hundred or so db reads on every request (see issue filed here https://github.com/dpgaspar/Flask-AppBuilder/issues/1039). The biggest contributor to these reads is the `/queries` API because it's polled every 2 seconds when a user has a query pending/running in SQL Lab. However, this API should be accessable to all logged in users as it simply returns the set of all queries the user owns. Therefore, I've introduced the new `logged_in_api` decorator to more efficiently secure this view. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> Gist of SQL logs for `/queries` endpoint before and after this PR: https://gist.github.com/etr2460/96c9ead2fda43aff507602519cc3183e Chart of queries against the superset database after deploying the fix on Tuesday afternoon:  ### TEST PLAN <!--- What steps should be taken to verify the changes --> - Run all unit tests to ensure the view is still secured. - Ensure you can hit the queries endpoint from SQL lab when logged in, but curling it without the cookie returns a 401 ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Changes UI - [ ] Requires DB Migration. - [ ] Confirm DB Migration upgrade and downgrade tested. - [ ] Introduces new feature or API - [ ] Removes existing feature or API ### REVIEWERS @john-bodley @mistercrunch @michellethomas @dpgaspar
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
