superset icon indicating copy to clipboard operation
superset copied to clipboard

feat(GAQ): Add Redis Sentinel Support for Global Async Queries

Open nsivarajan opened this issue 1 year ago • 6 comments

SUMMARY

This PR introduces a feature that allows the Async Query Manager to use a configured cache backend through the new GLOBAL_ASYNC_QUERIES_CACHE_BACKEND setting. To maintain backward compatibility, the existing GLOBAL_ASYNC_QUERIES_REDIS_CONFIG setting is still supported but will be deprecated in the future. Additionally, this update introduces support for Redis Sentinel caching, which eliminates the single point of failure associated with the previous standalone Redis configuration.

References taken from this Idea https://github.com/apache/superset/discussions/28839

TESTING INSTRUCTIONS

Configure GLOBAL_ASYNC_QUERIES_CACHE_BACKEND in config.py or superset_config.py with the appropriate properties: Set CACHE_TYPE to "RedisCache" for Redis cache backend. Set CACHE_TYPE to "RedisSentinelCache" for Redis Sentinel cache backend. Set CACHE_TYPE to None to fall back on the previous GLOBAL_ASYNC_QUERIES_REDIS_CONFIG.

ADDITIONAL INFORMATION

  • [ ] Has associated issue:
  • [ ] 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

nsivarajan avatar Aug 10 '24 17:08 nsivarajan

Codecov Report

:x: Patch coverage is 67.94872% with 25 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 83.64%. Comparing base (76d897e) to head (4cc558a). :warning: Report is 2203 commits behind head on master.

Files with missing lines Patch % Lines
superset/async_events/cache_backend.py 59.45% 15 Missing :warning:
superset/async_events/async_query_manager.py 69.69% 10 Missing :warning:
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #29912       +/-   ##
===========================================
+ Coverage   60.48%   83.64%   +23.15%     
===========================================
  Files        1931      529     -1402     
  Lines       76236    38268    -37968     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    32009    -14105     
+ Misses      28017     6259    -21758     
+ Partials     2105        0     -2105     
Flag Coverage Δ
hive ∅ <43.58%> (∅)
javascript ?
mysql ∅ <67.94%> (?)
postgres ∅ <67.94%> (?)
presto ∅ <61.53%> (∅)
python ∅ <67.94%> (∅)
sqlite ∅ <67.94%> (?)
unit ∅ <44.87%> (∅)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Aug 10 '24 20:08 codecov[bot]

HI @villebro , Could you please review this PR? It was created based on the idea discussed https://github.com/apache/superset/discussions/28839

However, I recently noticed that there is a new SIP in the design phase to modernize the framework around the same topic: #29839 . I'm not sure if this PR will still add value before the new proposal is rolled out, but I would appreciate your feedback.

nsivarajan avatar Aug 12 '24 14:08 nsivarajan

@nsivarajan thanks for the PR - I think this PR is welcomed even if there's ongoing work to modernize the async task framework. Looks generally good, but let me do a proper review shortly (either today or tomorrow)

villebro avatar Aug 12 '24 17:08 villebro

@villebro Thanks for the feedback! Looking forward to your review.

nsivarajan avatar Aug 15 '24 01:08 nsivarajan

Hi @villebro,

I wanted to follow up on this PR. I understand you're busy, but I was hoping to get your review when you have a moment. Please let me know if there's any additional information I can provide or if there are any specific areas you'd like me to address.

Thanks again for your time and feedback!

nsivarajan avatar Aug 22 '24 18:08 nsivarajan

@nsivarajan thank you for your patience in the meantime... good to see that CI is passing and there's no conflicts still, at least! :D

rusackas avatar Aug 22 '24 19:08 rusackas

Thanks, @villebro, for the feedback and type annotation tips!

I appreciate your guidance throughout this process. I'll go ahead and open a new PR to remove the old config option and will reference the task once it's created on the project board, so it’ll be ready when the 5.0 breaking window opens up. Looking forward to wrapping this up and moving on to the next steps!

nsivarajan avatar Aug 30 '24 18:08 nsivarajan

Thanks @nsivarajan, I really appreciate the help! Here's the project card: https://github.com/orgs/apache/projects/345?pane=issue&itemId=78081192

villebro avatar Aug 30 '24 19:08 villebro