Skip to content

feat(redis): add Redis Sentinel support for HA cache discovery#2618

Open
ex0a wants to merge 3 commits intomozilla:mainfrom
ex0a:feat/redis-sentinel
Open

feat(redis): add Redis Sentinel support for HA cache discovery#2618
ex0a wants to merge 3 commits intomozilla:mainfrom
ex0a:feat/redis-sentinel

Conversation

@ex0a
Copy link

@ex0a ex0a commented Feb 16, 2026

  • Add build_sentinel() function to discover Redis master via Sentinel
  • Parse redis-sentinel://host:port,host2:port2/master_name URLs
  • Query sentinels with SENTINEL GET-MASTER-ADDR-BY-NAME
  • Add comprehensive logging for debugging discovery issues
  • Support password auth and optional DB selection
  • Route sentinel URLs from endpoint config path in cache.rs

@sylvestre
Copy link
Collaborator

sorry, it conflicts

@ex0a
Copy link
Author

ex0a commented Feb 16, 2026

What would you suggest? I'm using this personally but thought it might help someone else

@AJIOB
Copy link
Contributor

AJIOB commented Feb 17, 2026

Hi @ex0a ,

You should rebase your changes to the latest master branch changes or just merge them to your branch - and resolve the conflicts.

Then you need to push your changes to your branch.

That's should be fine!

@AJIOB
Copy link
Contributor

AJIOB commented Feb 17, 2026

CC @Xuanwo

IMO, we should implement the Redis Sentinel support as a part of the OpenDAL. And just use it here.

Possibly, it's already done.

@ex0a ex0a force-pushed the feat/redis-sentinel branch from d088922 to 5288c2d Compare February 17, 2026 10:52
@ex0a
Copy link
Author

ex0a commented Feb 17, 2026

Thanks for the review! I've pushed updates addressing both points:
Feature gating: redis has been restored to the all feature list and the redis crate dependency is now properly optional via dep:redis, so it won't be compiled in unless the redis feature is enabled. I've also removed the hard tracing dependency I accidentally added.
Credentials warning: Added a warn!() when username, password, or db are provided alongside a redis-sentinel:// URL, since those fields are ignored in sentinel mode (credentials should be embedded in the URL itself).
Regarding OpenDAL: I considered this approach, but OpenDAL's Redis service doesn't currently support Sentinel natively. My implementation handles discovery at the sccache layer — querying sentinels to resolve the current master address, then building a normal single-node OpenDAL connection to it. This keeps the OpenDAL layer unchanged and avoids requiring upstream OpenDAL changes. Happy to revisit if OpenDAL adds Sentinel support in the future.

- Add build_sentinel() function to discover Redis master via Sentinel
- Parse redis-sentinel://host:port,host2:port2/master_name URLs
- Query sentinels with SENTINEL GET-MASTER-ADDR-BY-NAME
- Add comprehensive logging for debugging discovery issues
- Support password auth and optional DB selection
- Route sentinel URLs from endpoint config path in cache.rs
- Warn when username/password/db are provided with sentinel URLs
- Fix redis feature gating with dep:redis
@ex0a ex0a force-pushed the feat/redis-sentinel branch from 5288c2d to b477146 Compare February 17, 2026 10:59
@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 9.61538% with 94 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.54%. Comparing base (1b480c9) to head (b477146).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/cache/redis.rs 0.00% 88 Missing ⚠️
src/cache/cache.rs 62.50% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2618      +/-   ##
==========================================
- Coverage   72.74%   72.54%   -0.20%     
==========================================
  Files          68       68              
  Lines       36985    37079      +94     
==========================================
- Hits        26903    26900       -3     
- Misses      10082    10179      +97     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sylvestre
Copy link
Collaborator

please fix the jobs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants