Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slave when bound to localhost cannot connect to master #2612

Closed
echaozh opened this issue Jun 10, 2015 · 6 comments

Comments

@echaozh
Copy link

commented Jun 10, 2015

#2110 switched from anetTcpNonBlockConnect to anetTcpNonBlockBindConnect for the slave's connection to the master. If the slave is only bound to the lo interface, it will not be able to connect to masters on other machines.

I think it's useful to let the slave bound to the lo interface while connecting to masters on another machine. For my caching setup, I have 1 read/write master which can fail at any time and I don't really care, and a read-only slave on each machine the app is deployed. The slave is in an disk image and will be replicated by the cloud, so I don't want them each to be configured with a different LAN address. Also I don't want them to bind to 0.0.0.0 as the machines will have WAN access.

Hopefully we can revert back to non-binding connects to the master.

@antirez

This comment has been minimized.

Copy link
Owner

commented Jun 11, 2015

Hello. Ok the problem here is that there are times when we would like to connect using the same interface as the one we find, since the other end of the connection may get our socket name using the getpeername call. This is the case of the slave connection, for example. However this is not the case of MIGRATE, which is much more of just an error introduced during a refactoring around this area AFAIK. I remembered I reviewed the code, asked myself if it was the case or not to bind the same IF for bind, but then did not changed it.

So what is the solution? For MIGRATE it is probably just not needed to bind, but I'll check. For Slave I'm adding an API to anet so that it will use the right interface only if possible, otherwise will resort to what interface is possible to use in order to get a connection.

News in a few minutes hopefully.

@antirez

This comment has been minimized.

Copy link
Owner

commented Jun 11, 2015

That's the commit that changed the behavior: f1a6f78

I can see why @mattsta did this for the second function (connectWithMaster), but not for the first (migrateGetSocket). It was probably a grep effort in the aim to catch all the occurrences. I think it is safe to use anetTcpNonBlockConnect for MIGRATE. For the slave-master connection instead I'll write a best effort function that just attempts to bind the interface and otherwise uses a different one.

@antirez

This comment has been minimized.

Copy link
Owner

commented Jun 11, 2015

anetTcpGenericConnect had more issues btw, it jumped to the end label multiple times when there was actually a failure in binding the address, while it should jump to error AFAIK.

antirez added a commit that referenced this issue Jun 11, 2015
antirez added a commit that referenced this issue Jun 11, 2015
Two code paths jumped to the "ok, return the socket to the user" code
path to handle error conditions.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
This performs a best effort source address binding attempt. If it is
possible to bind the local address and still have a successful
connect(), then this socket is returned. Otherwise the call is retried
without source address binding attempt.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
We usually want to reach the master using the address of the interface
Redis is bound to (via the "bind" config option). That's useful since
the master will get (and publish) the slave address getting the peer
name of the incoming socket connection from the slave.

However, when this is not possible, for example because the slave is
bound to the loopback interface but repliaces from a master accessed via
an external interface, we want to still connect with the master even
from a different interface: in this case it is not really important that
the master will provide any other address, while it is vital to be able
to replicate correctly.

Related to issues #2609 and #2612.
@antirez

This comment has been minimized.

Copy link
Owner

commented Jun 11, 2015

Fixed, please confirm if you can.

antirez added a commit that referenced this issue Jun 11, 2015
antirez added a commit that referenced this issue Jun 11, 2015
Two code paths jumped to the "ok, return the socket to the user" code
path to handle error conditions.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
This performs a best effort source address binding attempt. If it is
possible to bind the local address and still have a successful
connect(), then this socket is returned. Otherwise the call is retried
without source address binding attempt.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
We usually want to reach the master using the address of the interface
Redis is bound to (via the "bind" config option). That's useful since
the master will get (and publish) the slave address getting the peer
name of the incoming socket connection from the slave.

However, when this is not possible, for example because the slave is
bound to the loopback interface but repliaces from a master accessed via
an external interface, we want to still connect with the master even
from a different interface: in this case it is not really important that
the master will provide any other address, while it is vital to be able
to replicate correctly.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
antirez added a commit that referenced this issue Jun 11, 2015
Two code paths jumped to the "ok, return the socket to the user" code
path to handle error conditions.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
This performs a best effort source address binding attempt. If it is
possible to bind the local address and still have a successful
connect(), then this socket is returned. Otherwise the call is retried
without source address binding attempt.

Related to issues #2609 and #2612.
antirez added a commit that referenced this issue Jun 11, 2015
We usually want to reach the master using the address of the interface
Redis is bound to (via the "bind" config option). That's useful since
the master will get (and publish) the slave address getting the peer
name of the incoming socket connection from the slave.

However, when this is not possible, for example because the slave is
bound to the loopback interface but repliaces from a master accessed via
an external interface, we want to still connect with the master even
from a different interface: in this case it is not really important that
the master will provide any other address, while it is vital to be able
to replicate correctly.

Related to issues #2609 and #2612.
@echaozh

This comment has been minimized.

Copy link
Author

commented Jun 24, 2015

It works, thanks.

@kbucheli

This comment has been minimized.

Copy link

commented Jul 7, 2015

I can confirm both the bug and that the bugfix is working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.