search

ledgetech / lua-resty-redis-connector

Connection utilities for lua-resty-redis
234 stars 71 forks source link

Fix issue retrieving available slaves via Sentinel #36

Closed ryaneorth closed 4 years ago

ryaneorth commented 4 years ago

A previous PR was submitted to fix this issue but it broke the tests and it was not clear whether the situation of a slave having a flag of s_down could occur when its master-link-status was ok. I have found that this is a situation that is very possible to get into as described in this comment.

This change updates the client to be on par with other languages' sentinel clients in terms of how slaves are determined to be healthy/available. See here and here as examples.

pintsized commented 4 years ago

Thanks for this, and the explanation. Looks good to me, though were you able to run the tests locally? I'm not working on this stuff at the moment so I don't have a local development environment.

I've been meaning to get CI working for this stuff for years though, I'll try and sort that out today and then we can look at merging from there.

ryaneorth commented 4 years ago

Thanks @pintsized ! Yes, I was able to run the tests locally and they pass.

pintsized commented 4 years ago

Ok, travis builds are now working on the master branch (only took about 1000 attempts!). I think if you rebase, travis should build this branch too?

ryaneorth commented 4 years ago

@pintsized - done and it looks like everything passed!