cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ifesdjeen <...@git.apache.org>
Subject [GitHub] cassandra pull request #269: Review tr range movements
Date Wed, 26 Sep 2018 07:43:45 GMT
Github user ifesdjeen commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/269#discussion_r220455656
  
    --- Diff: src/java/org/apache/cassandra/dht/RangeStreamer.java ---
    @@ -424,62 +440,58 @@ else if (useStrictConsistency)
                      EndpointsForRange sources;
                      if (useStrictConsistency)
                      {
    -                     //Start with two sets of who replicates the range before and who
replicates it after
    -                     EndpointsForRange newEndpoints = strat.calculateNaturalReplicas(toFetch.range().right,
tmdAfter);
    -                     logger.debug("Old endpoints {}", oldEndpoints);
    -                     logger.debug("New endpoints {}", newEndpoints);
    -
    +                     EndpointsForRange strictEndpoints;
                          //Due to CASSANDRA-5953 we can have a higher RF then we have endpoints.
                          //So we need to be careful to only be strict when endpoints == RF
                          if (oldEndpoints.size() == strat.getReplicationFactor().allReplicas)
                          {
    -                         Set<InetAddressAndPort> endpointsStillReplicated = newEndpoints.endpoints();
    +                         //Start with two sets of who replicates the range before and
who replicates it after
    +                         EndpointsForRange newEndpoints = strat.calculateNaturalReplicas(toFetch.range().right,
tmdAfter);
    +                         logger.debug("Old endpoints {}", oldEndpoints);
    +                         logger.debug("New endpoints {}", newEndpoints);
    +
                              // Remove new endpoints from old endpoints based on address
    -                         oldEndpoints = oldEndpoints.filter(r -> !endpointsStillReplicated.contains(r.endpoint()));
    -                         oldEndpoints.filter(testSourceFilters);
    +                         strictEndpoints = oldEndpoints.without(newEndpoints.endpoints());
     
    -                         if (oldEndpoints.size() > 1)
    -                             throw new AssertionError("Expected <= 1 endpoint but
found " + oldEndpoints);
    +                         //We have to check the source filters here to see if they will
remove any replicas
    +                         //required for strict consistency
    +                         if (!all(strictEndpoints, testSourceFilters))
    +                             throw new IllegalStateException("Necessary replicas for
strict consistency were removed by source filters: " + buildErrorMessage(sourceFilters, strictEndpoints));
    +
    +                         if (strictEndpoints.size() > 1)
    --- End diff --
    
    We're not really filtering anymore at all. We can get here only in case test source filters
didn't filter any nodes (otherwise we'd fail with the same assertion later). We've asserted
that the resulting endpoint is not going to be yanked from under us.


---

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


Mime
View raw message