Github user aweisberg commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/184#discussion_r163062356
  
    --- Diff: src/java/org/apache/cassandra/db/SystemKeyspace.java ---
    @@ -607,39 +672,65 @@ public static long getTruncatedAt(TableId id)
         /**
          * Record tokens being used by another node
          */
    -    public static synchronized void updateTokens(InetAddress ep, 
Collection<Token> tokens)
    +    public static synchronized void updateTokens(InetAddressAndPort ep, 
Collection<Token> tokens)
         {
    -        if (ep.equals(FBUtilities.getBroadcastAddress()))
    +        if (ep.equals(FBUtilities.getBroadcastAddressAndPort()))
                 return;
     
             String req = "INSERT INTO system.%s (peer, tokens) VALUES (?, ?)";
    -        executeInternal(format(req, PEERS), ep, tokensAsSet(tokens));
    +        executeInternal(String.format(req, LEGACY_PEERS), ep.address, 
tokensAsSet(tokens));
    +        req = "INSERT INTO system.%s (peer, peer_port, tokens) VALUES (?, 
?, ?)";
    +        executeInternal(String.format(req, PEERS_V2), ep.address, ep.port, 
tokensAsSet(tokens));
         }
     
    -    public static synchronized void updatePreferredIP(InetAddress ep, 
InetAddress preferred_ip)
    +    public static synchronized void updatePreferredIP(InetAddressAndPort 
ep, InetAddressAndPort preferred_ip)
         {
             if (getPreferredIP(ep) == preferred_ip)
                 return;
     
             String req = "INSERT INTO system.%s (peer, preferred_ip) VALUES 
(?, ?)";
    -        executeInternal(format(req, PEERS), ep, preferred_ip);
    -        forceBlockingFlush(PEERS);
    +        executeInternal(String.format(req, LEGACY_PEERS), ep.address, 
preferred_ip.address);
    +        forceBlockingFlush(LEGACY_PEERS);
    --- End diff --
    
    Well... we do this in a lot of places. Some things are called indirectly. I 
think we can add a bit of complexity to solve a problem we aren't sure we have.
    
    Also is this really an issue with this patch which yes increases the number 
of places we do this or is it out of scope?
    
    I've already implemented, but going through and seeing the number of places 
where I wouldn't be applying this due to indirection was disappointing.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to