Skip to content

Validate Addr, Exclude nodes with invalid ip address#95

Open
nidhi-ag wants to merge 3 commits intojoyieldInc:masterfrom
nidhi-ag:master
Open

Validate Addr, Exclude nodes with invalid ip address#95
nidhi-ag wants to merge 3 commits intojoyieldInc:masterfrom
nidhi-ag:master

Conversation

@nidhi-ag
Copy link
Copy Markdown

@nidhi-ag nidhi-ag commented Nov 13, 2019

Problem Statement

Recently we faced an issue with predixy during redis cluster scaling, where it was trying to connect to empty host.

Incidents

  • PHP Redis client threw error message "read error on connection".

  • On checking predixy logs we found
    2019-11-12 08:05:03.498123 W Handler.cpp:396 h 4 c 127.0.0.1:47187 430 handle event 1 exception Socket.cpp:120 invalid addr :6379:Name or service not known
    where addr is empty

live-predixy-error-logs

  • Predixy INFO command output
    predixy-invalid-info
    here one of the slaves are registered in the server pool even though it's host is invalid.

After replicating this issue on a test cluster, we found out that at some point "cluster nodes" is returning an invalid response where it doesn't have addr though the flag is valid (slave).
D ClusterServerPool.cpp:103 redis cluster update parse node 52a144f80835728846892506bbd7942ac49bf09c :6379@1122 slave -

Screenshot 2019-11-12 at 6 59 37 PM

Solution

  • To handle this case this pull request adds address validation step to discard such invalid response from the redis.

Although the redis should respond according to the protocol, I think if the redis responds with an invalid format, predixy should do validations before inserting into the pool.

@nidhi-ag nidhi-ag changed the title [WIP] Validate Addr, Exclude nodes with invalid ip address Validate Addr, Exclude nodes with invalid ip address Nov 15, 2019
if (port) {
std::string tmp;
tmp.append(mAddr, port - mAddr);
host = tmp.c_str();
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.

the host will point an invalid address

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now i have changed it to use mAddr data instead of pointer.

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.

2 participants