Patch (generated by 'svn diff' on revision 760988)

The patch adds a new option to storage-conf.xml
<LocalIp>127.0.0.1</LocalIp>

The option is used to indicate on which local ip-address to bind.
This can be used to run the server on internal vs external ip-addresses etc.

Note that one use of this is runnng several instances on the same machine for local testing.
One config file to run on 127.0.0.1 another one to run on 127.0.0.2 etc

Per, you should join us in #cassandra on freenode irc. http://www.mibbit.com/?server=irc.freenode.net&channel=%23cassandra&nick=mibbit if you don't have an irc client.

IMO if you're going to start mixing IPs in you should clean out the old hostname-based stuff entirely. (If no IP is given then pick one.)

New patch instead of the first one.

This patch removes all the host-based stuff. If a <LocalIp> setting is present in storage-conf.xml that ip-address will be used. Otherwise cassandra will use the local ip-address of the server IF there is only one (not counting loopback). If the server has multiple ip-addresses cassandra must be told which one to use in storage-conf.xml.

I'm having some trouble applying this patch. I think the error message, "patch: **** malformed patch at line 84: @@ -20,6 +20,7 @@" is a red herring. The actual problem seems to be with the hunk preceding it (for src/org/apache/cassandra/net/EndPoint.java).

Per, can you try regenerating the patch?

After beating my head repeatedly on some weird mismatch between svn diff and patch this diff file now contains all changes EXCEPT the changes for EndPoint.java. Those changes are in a separate patch file that will have to be applied manually as patch complains about the diff file syntax.

Separate diff file for EndPoint.java
This change has to be applied manually due to some mismatch between svn diff and patch

First, a few minor issues/styling nits:

This patch makes the LocalIP config required, so test/conf/storage-conf.xml should be updated accordingly or the tests fail.

Speaking of the directive, I can see where you got the name "LocalIP", but its purpose might be more obvious if it were named something like "ListenInterface" or "BindInterface". Or "ListenAddress" maybe?

If FBUtilities.isHostLocalHost() is no longer needed, it probably makes sense to just remove it from the source altogether instead of commenting it out (it can always be retrieved from history later if needed).

You should use a space between operators in your expressions, since that is the convention used throughout the project.

Now, as to the functionality:

The way the patch is setup, it's expecting that a) either the address has been explicitly configured, or b) that it is able to find at least one IPv4 interface with a non-loopback address (and it will run with the last one it finds). If neither condition is met, an exception is thrown and startup is prevented.

I'm not sure this is the best behavior. Firstly, I'd hate to start down a path that led to no support for IPv6. Secondly, in the absence of a configuration, address selection is less deterministic than what is in trunk (depends on order of interface/address enumeration versus the host network configuration).

I think this needs to take one of two routes:

1. Everyone configures the interface to bind to. No exceptions. Throw an exception for missing or invalid configuration.

2. The default (read: trunk) behavior is retained (i.e. InetAddress.getLocalHost()), which can be overridden by configuration.

Personally, I favor #2 since it is the element of least surprise to those who already have working clusters.

Also, jbellis raised a valid point on IRC that DNS is a point of failure and shouldn't be mandatory.

I don't see any reason why you couldn't retain the behavior of this patch with respect to returning a string representation of the selected IP address.

Attached are two new patches.

The first fixes the crazy line endings and indentation in EndPoint.java that have been making patch generation such a pain. The second is basically Per's first patch, rebased against current trunk, and with the configuration directive renamed to "ListenAddress".

Per, is this ok with you?

applied Eric's tweaks to Per's first patch.

Reverted 0002 since it prevents server from starting up. Haven't debugged more than that.

back to letting thrift set reuseaddr etc. centralize ListenAddress logic in getHostName.

looks like I forgot to resolve this last week.