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

    https://github.com/apache/cassandra/pull/224#discussion_r189119609
  
    --- Diff: src/java/org/apache/cassandra/service/reads/DataResolver.java ---
    @@ -64,12 +72,19 @@ public PartitionIterator resolve()
             // at the beginning of this method), so grab the response count 
once and use that through the method.
             int count = responses.size();
             List<UnfilteredPartitionIterator> iters = new ArrayList<>(count);
    -        InetAddressAndPort[] sources = new InetAddressAndPort[count];
    +        Replica[] sources = new Replica[count];
             for (int i = 0; i < count; i++)
             {
                 MessageIn<ReadResponse> msg = responses.get(i);
                 iters.add(msg.payload.makeIterator(command));
    -            sources[i] = msg.from;
    +
    +            Replica replica = replicaMap.get(msg.from);
    +            if (replica == null)
    --- End diff --
    
    My thinking here was that the speculative read repair patch (in review 
still) could speculate to a node right before a ring change, and then wouldn't 
be able to find it through the read command. Since the speculation and read 
responses will be on different threads, adding replicas post DataResolver 
instantiation would require synchronization of the map, which isn't really 
worth it for the vanishingly small chance of this race occurring. The 
consequence of using a fake full replica is just the potential sending of read 
repair mutations to a transient replica.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org

Reply via email to