Skip to content

fix(celery cache warmup): add auth and use warm_up_cache endpoint - #21076

Merged
nytai merged 2 commits into
apache:masterfrom
nytai:tai/fix-celery-cache
Aug 30, 2022
Merged

nytai merged 2 commits into
apache:masterfrom
nytai:tai/fix-celery-cache

Conversation

@nytai

@nytai nytai commented Aug 12, 2022 •

Copy link
Copy Markdown
Member

SUMMARY

Fixes cache warmup via celery job.

  • adds cache and data cache configs to the docker-compose superset_config to enable caching for development env.
  • add authentication to the cache warmup celery beat job, using the THUMBNAIL_SELENIUM_USER config for auth.
  • use the /superset/warm_up_cache endpoint for warming up the cache.
  • run the warmup call as a single celery job instead of running all the warmups in single job. This approach should enable some parallelization of the warm up calls. A slow warmup call should also not slow down the entire process.

There is currently an open PR that addresses this issue, however it is using a web driver process to compute the warmup by visiting the dashboard which seems a bit process heavy. Since there is already a cache warmup endpoint that is able to handle charts and filters combos, it make more sense to use that endpoint.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  • use the docker-compose workflow
  • Add a celery cache warmup beat config using top_n_strategy, set it to run at a high interval (for eg, every minute). documented here
  • visit dashboards to that top n strategy has data to work with
  • observe the cache warmup logs in the worker
  • visit dashboards and notice the charts are cached

ADDITIONAL INFORMATION

  • Has associated issue: Fixes warm up cache does not work #18933, Fixes Cache warm-ups never succeed #9597
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@nytai
nytai marked this pull request as ready for review August 12, 2022 19:08
@codecov

codecov Bot commented Aug 12, 2022 •

Copy link
Copy Markdown

Codecov Report

Merging #21076 (088ab7d) into master (5113b01) will decrease coverage by 0.05%.
The diff coverage is 34.28%.

❗ Current head 088ab7d differs from pull request most recent head e233d65. Consider uploading reports for the commit e233d65 to get more accurate results

@@            Coverage Diff             @@
##           master   #21076      +/-   ##
==========================================
- Coverage   66.28%   66.23%   -0.06%     
==========================================
  Files        1770     1770              
  Lines       67522    67528       +6     
  Branches     7177     7177              
==========================================
- Hits        44754    44724      -30     
- Misses      20934    20970      +36     
  Partials     1834     1834              
Flag Coverage Δ
hive 53.16% <17.14%> (-0.01%) ⬇️
mysql ?
postgres 80.96% <34.28%> (-0.05%) ⬇️
presto 53.06% <17.14%> (-0.01%) ⬇️
python 81.38% <34.28%> (-0.11%) ⬇️
sqlite 79.56% <34.28%> (-0.05%) ⬇️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/tasks/cache.py 61.15% <34.28%> (-12.76%) ⬇️
superset/common/utils/dataframe_utils.py 90.47% <0.00%> (-4.77%) ⬇️
superset/db_engine_specs/mysql.py 94.04% <0.00%> (-4.77%) ⬇️
superset/initialization/__init__.py 89.96% <0.00%> (-1.74%) ⬇️
superset/models/core.py 88.03% <0.00%> (-0.72%) ⬇️
superset/views/core.py 75.15% <0.00%> (-0.46%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nytai
nytai force-pushed the tai/fix-celery-cache branch from bd797a5 to e7f62e1 Compare August 12, 2022 20:27
@nytai
nytai force-pushed the tai/fix-celery-cache branch from 6cf8739 to e233d65 Compare August 12, 2022 20:48
@zhaoyongjie
zhaoyongjie self-requested a review August 15, 2022 15:58

@geido geido left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nytai

nytai commented Aug 30, 2022

Copy link
Copy Markdown
Member Author

rerunning CI

@nytai nytai closed this Aug 30, 2022
@nytai nytai reopened this Aug 30, 2022
@nytai
nytai merged commit 04dd8d4 into apache:master Aug 30, 2022
@nytai
nytai deleted the tai/fix-celery-cache branch August 30, 2022 16:24
michael-s-molina pushed a commit that referenced this pull request Sep 6, 2022
@nytai

nytai commented Sep 7, 2022

Copy link
Copy Markdown
Member Author

UPDATE: I've recently discovered that the /superset/warm_up_cache endpoint does not work with the new charts based on the /api/v1/chart/data api and with dashboards that are using dashboard native filters. Generating a query context in the backend only will be quite difficult, so the approach in #20387 might be better

eschutho pushed a commit that referenced this pull request Sep 20, 2022
Fahrenheit35 pushed a commit to Fahrenheit35/superset that referenced this pull request Nov 11, 2022
Usiel added a commit to Usiel/superset that referenced this pull request May 10, 2023
We stopped using SUPERSET_WEBSERVER_PROTOCOL, ..._ADDRESS, ..._PORT with apache#21076. Removing the config from the example config as there does not seem to be any immediate need for it, we already have WEBDRIVER_BASEURL (and the _USER_FRIENDLY variant).
Usiel added a commit to Usiel/superset that referenced this pull request Feb 1, 2024
We stopped using SUPERSET_WEBSERVER_PROTOCOL, ..._ADDRESS, ..._PORT with apache#21076. Removing the config from the example config as there does not seem to be any immediate need for it, we already have WEBDRIVER_BASEURL (and the _USER_FRIENDLY variant).
@mistercrunch mistercrunch added 🍒 1.5.2 Cherry-picked to 1.5.2 🍒 1.5.3 Cherry-picked to 1.5.3 🍒 2.0.1 Cherry-picked to 2.0.1 🚢 2.1.3 First shipped in 2.1.3 🚢 2.1.0 First shipped in 2.1.0 and removed 🍒 2.0.1 Cherry-picked to 2.0.1 🍒 1.5.3 Cherry-picked to 1.5.3 🍒 1.5.2 Cherry-picked to 1.5.2 🚢 2.1.3 First shipped in 2.1.3 🚢 2.1.0 First shipped in 2.1.0 labels Feb 18, 2024
@mistercrunch mistercrunch added 🍒 1.5.2 Cherry-picked to 1.5.2 🍒 1.5.3 Cherry-picked to 1.5.3 🍒 2.0.1 Cherry-picked to 2.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 First shipped in 2.1.0 and removed 🚢 2.1.3 First shipped in 2.1.3 labels Mar 13, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
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 size/L v2.0 v2.0.1 🍒 1.5.2 Cherry-picked to 1.5.2 🍒 1.5.3 Cherry-picked to 1.5.3 🍒 2.0.1 Cherry-picked to 2.0.1 🚢 2.1.0 First shipped in 2.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

warm up cache does not work Cache warm-ups never succeed

5 participants