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

[sql lab] always use NullPool #5612

Merged
merged 1 commit into from
Aug 14, 2018
Merged

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Aug 13, 2018

I think that the only place where we want db connection pooling would be
to talk to the metadata database. SQL Lab should close its connections
and never pool them.
Given that each Gunicorn worker will create its own pool that can lead
to way too many connections opened.

closes #4666

@john-bodley @betodealmeida

I think that the only place where we want db connection pooling would be
to talk to the metadata database. SQL Lab should close its connections
and never pool them.
Given that each Gunicorn worker will create its own pool that can lead
to way too many connections opened.

closes apache#4666
@codecov-io
Copy link

Codecov Report

Merging #5612 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5612   +/-   ##
=======================================
  Coverage   63.62%   63.62%           
=======================================
  Files         359      359           
  Lines       22820    22820           
  Branches     2533     2533           
=======================================
  Hits        14520    14520           
  Misses       8285     8285           
  Partials       15       15
Impacted Files Coverage Δ
superset/sql_lab.py 71.77% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50981db...91d7684. Read the comment docs.

@timifasubaa
Copy link
Contributor

@mistercrunch
Copy link
Member Author

@timifasubaa I think it's out of the scope of this PR. Also it looks like csv isn't using the Database.get_sqla_engine method as it probably should. If it was then the default is nullpool=True

@mistercrunch mistercrunch merged commit be04c98 into apache:master Aug 14, 2018
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
I think that the only place where we want db connection pooling would be
to talk to the metadata database. SQL Lab should close its connections
and never pool them.
Given that each Gunicorn worker will create its own pool that can lead
to way too many connections opened.

closes apache#4666
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sessions still active on Datasource
4 participants