Skip to content

feat(ring): specify custom health check func via HeartbeatFn option#2940

Merged
ndyakov merged 11 commits into
redis:masterfrom
strobil:feature/custom-ring-shard-hc-func
Aug 5, 2025
Merged

feat(ring): specify custom health check func via HeartbeatFn option#2940
ndyakov merged 11 commits into
redis:masterfrom
strobil:feature/custom-ring-shard-hc-func

Conversation

@strobil

@strobil strobil commented Mar 13, 2024

Copy link
Copy Markdown
Contributor

The current implementation of the Redis ring does not support specifying custom shard health check rules.
We use a Redis shard to distribute the load across multiple Redis slave instances.
However, if any shard encounters replication issues, the Redis ring continues to utilize this shard because it responds to ping requests successfully.
Thus, the Redis ring does not remove the malfunctioning shard, even though it is inconsistent.

This pull request proposes adding a HeartbeatFn option to the RingOptions, which can be configured during ring construction. This addition enables the definition of custom health check logic, such as checks based on the current replication status of the shard.

@strobil strobil marked this pull request as draft March 13, 2024 22:19
@strobil strobil marked this pull request as ready for review March 14, 2024 10:42
@strobil strobil changed the title specify custom health check func via ShardHealthCheckFn option specify custom health check func via HeartbeatFn option Mar 14, 2024
@ndyakov

ndyakov commented Feb 4, 2025

Copy link
Copy Markdown
Member

Thank you, @strobil! I do think having a custom healthcheck in the ring can benefit the users. Would you mind resolving conflicts so we can merge master and proceed with review of this PR?

@strobil strobil changed the title specify custom health check func via HeartbeatFn option feat(ring): specify custom health check func via HeartbeatFn option Feb 21, 2025
@strobil

strobil commented Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

@ndyakov
Hi, all merge conflicts were resolved!

@ndyakov

ndyakov commented Aug 4, 2025

Copy link
Copy Markdown
Member

@strobil i do see the test continue to fail, would you mind taking a look

@ndyakov ndyakov self-requested a review August 5, 2025 11:59

@ndyakov ndyakov 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.

Approving this. @strobil keep in mind we are not actively supporting ring since there are other limitation that simply cannot be covered with clustering controlled from the client.

@ndyakov ndyakov merged commit 7158a8d into redis:master Aug 5, 2025
20 checks passed
ofekshenawa added a commit that referenced this pull request Aug 10, 2025
…2940)

* specify custom health check func via ShardHealthCheckFn option

* ShardHealthCheckFn renamed to HeartbeatFn

---------

Co-authored-by: Mykhailo Alipa <strobil@Mykhailos-MacBook-Air.local>
Co-authored-by: ofekshenawa <104765379+ofekshenawa@users.noreply.github.com>
Co-authored-by: Nedyalko Dyakov <nedyalko.dyakov@gmail.com>
Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants