phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [phoenix-omid] stoty commented on a change in pull request #62: OMID-156 refactor Omid to use phoenix-shaded-guava
Date Tue, 11 Aug 2020 07:08:57 GMT

stoty commented on a change in pull request #62:
URL: https://github.com/apache/phoenix-omid/pull/62#discussion_r468369903



##########
File path: common/src/main/java/org/apache/omid/NetworkUtils.java
##########
@@ -34,6 +36,14 @@
 
     public static String getDefaultNetworkInterface() {
 
+        try (DatagramSocket s=new DatagramSocket()) {

Review comment:
       We specifically don't want to bind to localhost, we want to find to the "public" network
interface that the clients can access.
   
   The idea is that a random public IP address will be routed through the default route, which
goes through the "public" network interface, and the clients will be able to access the server
on the IP address of this "public' interface.
   
   This is all just hacky guesswork as there is no way to definitely know which interface
we want. What we really should do is make the public address configurable.
   
   As for the actual implementation:
   
   - The DatagramSocket() constructor will bind to 0.0.0.0, the wildcard address. (though
we don't really care about the listening part)
   - connect() will change the internal state of the Socket to only allow traffic to/from
that address. We don't really care about that, what we do care about is that as a side effect,
it will set the socket's localAddress to that of the outgoing interface that 1.1.1.1 is accessible
via, and which we hope is the "public interface"
   - After this, we will close the socket without having transmitted or received a single
packet on it.
   
   Revisiting this, we'd probably have a better chance of finding the right interface if we
used an address from the ZK ensemble instead of 1.1.1.1, in case there's some weird networking
setup in the cluster.
   
   Furthermore, I think that the method of trying to figure out the interface, and then looking
up the address of that interface is also flawed, as interfaces may have multiple addressed
bound to them. A better approach would be using s.getLocalAddress() directly. 
   
   Completely rewriting that logic is outside the scope of this ticket, though.
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message