> > I think the pragmatic thing to do is fix it now, and I'd strongly > prefer to do that but wanted to check if there are any objections or > things I hadn't considered?
+1 from me, should fix. On Wed, Jul 29, 2020 at 11:59 AM Jon Meredith <jmeredit...@gmail.com> wrote: > Following up on Mick's email about interface incompatible changes, > CASSANDRA-15937 is ready for review. In my opinion, this just fixes > some bugs in CASSANDRA-7544 patch that slipped through the original > review, but before we review & merge fixes I wanted to ask if anybody > had any objections to merging it at this stage. > > The ticket has more details, but to recap when InetAddressAndPort was > introduced in CASSANDRA-7544 so that nodes can share the same IP > address with different storage ports, new versions of status Mbeans > were added that included the port so as not to break anything that > didn't expect a port number. Unfortunately the change introduced > inconsistencies in how the host address is formatted between the old > and new versions. It also updated some per-connection metrics which > are also affected. > > Before CASSANDRA-7544, describing host addresses either used > InetAddress.toString which formats as "<resolved>/<numeric>" or > InetAddress.getHostAddress which just provides a numeric address. > InetAddressAndPort was introduced with only a toString() method that > just provided the numeric address and port number, but did not provide > an equivalent getHostAddress/getHostAddressAndPort methods. The > updated versions of all the JMX endpoints all called toString and > dropped the resolved names. > > CASSANDRA-15937 adds an implementation of getHostAddressAndPort and > reverts the getHostAddress to toString conversions, giving consistent > formatting between the two versions, but with the port number. This > has the following impact (with a couple of other minor bugs fixed). > > 1) AllEndpointStats, SimpleStates, Connection message tracking, > TimeoutsPerHost now include the [host]/ip:port in the WithPort version > 2) Correct LoadMap to omit port (fixes backward compatibility) > LoadMapWithPort to include port (which was missing) > 3) Ownership - update WithPort to include the [host]/ip:port version > 4) Scores - update WithPort to [host]/ip:port version > > It's a shame we didn't discover/resolve it before we cut beta1 as it > does change things by introducing "[host]/" in some places, however > this is only in 4.0 related fields that were just introduced. > > The alternatives to merging I can see are > > 1) Do nothing for the 4.0 release and have the new WithPort versions > broken until we fix in a new major. > 2) Introduce a third-version of the naming with things fixed > (WithPort_2 or something awful) > 3) Reduce scope - don't 'fix' all the bare host addresses, just fix > LoadMap as it is the only one that is actually *wrong*, rather than > different. Perhaps the resolved name before the slash is annoying to > people. > > I think the pragmatic thing to do is fix it now, and I'd strongly > prefer to do that but wanted to check if there are any objections or > things I hadn't considered? > > Cheers, Jon. > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org > For additional commands, e-mail: dev-h...@cassandra.apache.org > >
