cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From belliottsmith <...@git.apache.org>
Subject [GitHub] cassandra pull request #265: 14705
Date Wed, 12 Sep 2018 20:19:45 GMT
Github user belliottsmith commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/265#discussion_r217174597
  
    --- Diff: src/java/org/apache/cassandra/service/reads/AbstractReadExecutor.java ---
    @@ -196,7 +196,7 @@ public static AbstractReadExecutor getReadExecutor(SinglePartitionReadCommand
co
     
             // There are simply no extra replicas to speculate.
             // Handle this separately so it can record failed attempts to speculate due to
lack of replicas
    -        if (replicaPlan.contact().size() >= replicaPlan.liveOnly().all().size())
    +        if (replicaPlan.contact().size() == replicaPlan.candidates().size())
    --- End diff --
    
    I modified to `>=` in an earlier version of this patch, and decided it was a silly
change and rolled it back.  Pretty much by definition, if we select `contact` from `candidates`,
it should always be `<=`.  I guess we could insert an assertion, but at some point we have
to drawn the line on what we verify and what we do not.  I'm willing to put an assertion in,
but I feel it is unnecessary, given we produce almost all `contact` via a call to `candidates.filter`


---

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


Mime
View raw message