Global Async Queries with Redis Sentinel #28839
Replies: 19 comments
This comment was marked as off-topic.
This comment was marked as off-topic.
-
So, Is Redis sentinel the only or the recommended choice for superset? |
Beta Was this translation helpful? Give feedback.
-
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue |
Beta Was this translation helpful? Give feedback.
-
I'm guessing we can close this issue as (a) stale, and (b) a feature request more than a bug. @himanshugarg574 are you still interested in this feature? If so, we can migrate this issue to a feature request Discussion. CC'ing/assigning @villebro in case he'd like to make a decision on closing/handling this issue, since he's taken an interest in this feature area. |
Beta Was this translation helpful? Give feedback.
-
Sentinel support here would be a great feature, currently unable to use Async queries due to this limitation. |
Beta Was this translation helpful? Give feedback.
-
Anyone on the thread here willing to contribute this change? This is kind of on the fence between being a bug and a feature request. We're trying to move feature requests to Github discussions so that we have a more actionable backlog of actual bugs here. I can: |
Beta Was this translation helpful? Give feedback.
-
IMO it seems like a bug, since Sentinel is otherwise supported with Superset. It took a bit of digging for me to realize that plain redis is hard-coded in the config for async queries |
Beta Was this translation helpful? Give feedback.
-
Cool... we'll just leave this open for now then, but if you want to open a PR, that would be awesome — I don't suspect it'll get fixed anytime soon otherwise. |
Beta Was this translation helpful? Give feedback.
-
@xenago oh yeah, I see what you mean. Fixing this should be straight forward. would you mind taking a stab at this? I assume we could just replace |
Beta Was this translation helpful? Give feedback.
-
We're trying to avoid opportunistic breaking changes, but it's now on the board to be considered for 5.0 :) The PR can most certainly be opened now, we'll just put a |
Beta Was this translation helpful? Give feedback.
-
I don't know if my understanding is 100 percent correct, but in Maybe either an instance of |
Beta Was this translation helpful? Give feedback.
-
I may also be hazy on the details, but IIRC, |
Beta Was this translation helpful? Give feedback.
-
Ah ok, thanks. I will test later this week to see if it works with a small patch to pass through a directly instantiated |
Beta Was this translation helpful? Give feedback.
-
A simple change would be adding something like this: GLOBAL_ASYNC_QUERIES_REDIS_CLASS = redis.Redis and then later use that, rather than the hardcoded class. However, IMO the more elegant solution would be make the backend configurable, similar to GLOBAL_ASYNC_QUERIES_BACKEND: BaseCache | None = None that could then be overridden locally: GLOBAL_ASYNC_QUERIES_BACKEND = Sentinel(...) Both should be backwards compatible (in the second option we could just deprecate |
Beta Was this translation helpful? Give feedback.
-
As a quick check, I edited |
Beta Was this translation helpful? Give feedback.
-
Ah, I spoke too soon. The
|
Beta Was this translation helpful? Give feedback.
-
@xenago @villebro do we want to keep this open as a Superset bug, or is this a bit of a "feature request" edge case? I'm quite tempted to close this as stale, but wanted to check in with y'all first. |
Beta Was this translation helpful? Give feedback.
-
Feature request sounds appropriate I think |
Beta Was this translation helpful? Give feedback.
-
@rusackas not sure if the team has tested this but: CACHE_CONFIG = { Reports:
|
Beta Was this translation helpful? Give feedback.
-
Is your feature request related to a problem? Please describe.
We want to use Redis sentinel with Global async query feature. Currently this feature only supports standalone redis instance which then becomes single point of failure.
Describe the solution you'd like
We can either pass RedisSentinelCache/RedisCache instance to config or can have additional parameters for supporting sentinel.
Describe alternatives you've considered
Standalone Redis works but it is not fault tolerant.
Additional context
Beta Was this translation helpful? Give feedback.
All reactions