[incubator-superset] branch master updated: Using a NullPool for external connections by default (#4251)

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 4b11f45  Using a NullPool for external connections by default (#4251)
4b11f45 is described below

commit 4b11f45f72dc3d1957d6db4e0147fcd6cb6c59af
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Tue Jan 23 15:13:50 2018 -0800

    Using a NullPool for external connections by default (#4251)
    
    Currently, even though `get_sqla_engine` calls get memoized, engines are
    still short lived since they are attached to an models.Database ORM
    object. All engines created through this method have the scope of a web
    request.
    
    Knowing that the SQLAlchemy objects are short lived means that
    a related connection pool would also be short lived and mostly useless.
    I think it's pretty rare that connections get reused within the context
    of a view or Celery worker task.
    
    We've noticed on Redshift that Superset was leaving many connections
    opened (hundreds). This is probably due to a combination of the current
    process not garbage collecting connections properly, and perhaps the
    absence of connection timeout on the redshift side of things. This
    could also be related to the fact that we experience web requests timeouts
    (enforced by gunicorn) and that process-killing may not allow SQLAlchemy
    to clean up connections as they occur (which this PR may not help
    fixing...)
    
    For all these reasons, it seems like the right thing to do to use
    NullPool for external connection (but not for our connection to the metadata
    db!).
    
    Opening the PR for conversation. Putting this query into our staging
    today to run some tests.
---
 superset/models/core.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/models/core.py b/superset/models/core.py
index 413f85b..9f26a27 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -639,7 +639,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
 
     @utils.memoized(
         watch=('impersonate_user', 'sqlalchemy_uri_decrypted', 'extra'))
-    def get_sqla_engine(self, schema=None, nullpool=False, user_name=None):
+    def get_sqla_engine(self, schema=None, nullpool=True, user_name=None):
         extra = self.get_extra()
         url = make_url(self.sqlalchemy_uri_decrypted)
         url = self.db_engine_spec.adjust_database_uri(url, schema)

-- 
To stop receiving notification emails like this one, please contact
maximebeauchemin@apache.org.