cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aweisberg <...@git.apache.org>
Subject [GitHub] cassandra pull request #224: 14405 replicas
Date Wed, 16 May 2018 17:06:05 GMT
Github user aweisberg commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/224#discussion_r188102161
  
    --- Diff: src/java/org/apache/cassandra/locator/PendingRangeMaps.java ---
    @@ -23,196 +23,176 @@
     import com.google.common.collect.Iterators;
     import org.apache.cassandra.dht.Range;
     import org.apache.cassandra.dht.Token;
    -import org.slf4j.Logger;
    -import org.slf4j.LoggerFactory;
     
     import java.util.*;
     
    -public class PendingRangeMaps implements Iterable<Map.Entry<Range<Token>,
List<InetAddressAndPort>>>
    +public class PendingRangeMaps implements Iterable<Map.Entry<Range<Token>,
List<Replica>>>
     {
    -    private static final Logger logger = LoggerFactory.getLogger(PendingRangeMaps.class);
    -
         /**
          * We have for NavigableMap to be able to search for ranges containing a token efficiently.
          *
          * First two are for non-wrap-around ranges, and the last two are for wrap-around
ranges.
          */
         // ascendingMap will sort the ranges by the ascending order of right token
    -    final NavigableMap<Range<Token>, List<InetAddressAndPort>> ascendingMap;
    +    private final NavigableMap<Range<Token>, List<Replica>> ascendingMap;
    +
         /**
          * sorting end ascending, if ends are same, sorting begin descending, so that token
(end, end) will
          * come before (begin, end] with the same end, and (begin, end) will be selected
in the tailMap.
          */
    -    static final Comparator<Range<Token>> ascendingComparator = new Comparator<Range<Token>>()
    -        {
    -            @Override
    -            public int compare(Range<Token> o1, Range<Token> o2)
    -            {
    -                int res = o1.right.compareTo(o2.right);
    -                if (res != 0)
    -                    return res;
    +    private static final Comparator<Range<Token>> ascendingComparator = (o1,
o2) -> {
    +        int res = o1.right.compareTo(o2.right);
    +        if (res != 0)
    +            return res;
     
    -                return o2.left.compareTo(o1.left);
    -            }
    -        };
    +        return o2.left.compareTo(o1.left);
    +    };
     
         // ascendingMap will sort the ranges by the descending order of left token
    -    final NavigableMap<Range<Token>, List<InetAddressAndPort>> descendingMap;
    +    private final NavigableMap<Range<Token>, List<Replica>> descendingMap;
    +
         /**
          * sorting begin descending, if begins are same, sorting end descending, so that
token (begin, begin) will
          * come after (begin, end] with the same begin, and (begin, end) won't be selected
in the tailMap.
          */
    -    static final Comparator<Range<Token>> descendingComparator = new Comparator<Range<Token>>()
    -        {
    -            @Override
    -            public int compare(Range<Token> o1, Range<Token> o2)
    -            {
    -                int res = o2.left.compareTo(o1.left);
    -                if (res != 0)
    -                    return res;
    +    private static final Comparator<Range<Token>> descendingComparator =
(o1, o2) -> {
    +        int res = o2.left.compareTo(o1.left);
    +        if (res != 0)
    +            return res;
     
    -                // if left tokens are same, sort by the descending of the right tokens.
    -                return o2.right.compareTo(o1.right);
    -            }
    -        };
    +        // if left tokens are same, sort by the descending of the right tokens.
    +        return o2.right.compareTo(o1.right);
    +    };
     
         // these two maps are for warp around ranges.
    -    final NavigableMap<Range<Token>, List<InetAddressAndPort>> ascendingMapForWrapAround;
    +    private final NavigableMap<Range<Token>, List<Replica>> ascendingMapForWrapAround;
    +
         /**
          * for wrap around range (begin, end], which begin > end.
          * Sorting end ascending, if ends are same, sorting begin ascending,
          * so that token (end, end) will come before (begin, end] with the same end, and
(begin, end] will be selected in
          * the tailMap.
          */
    -    static final Comparator<Range<Token>> ascendingComparatorForWrapAround
= new Comparator<Range<Token>>()
    -    {
    -        @Override
    -        public int compare(Range<Token> o1, Range<Token> o2)
    -        {
    -            int res = o1.right.compareTo(o2.right);
    -            if (res != 0)
    -                return res;
    +    private static final Comparator<Range<Token>> ascendingComparatorForWrapAround
= (o1, o2) -> {
    +        int res = o1.right.compareTo(o2.right);
    +        if (res != 0)
    +            return res;
     
    -            return o1.left.compareTo(o2.left);
    -        }
    +        return o1.left.compareTo(o2.left);
         };
     
    -    final NavigableMap<Range<Token>, List<InetAddressAndPort>> descendingMapForWrapAround;
    +    private final NavigableMap<Range<Token>, List<Replica>> descendingMapForWrapAround;
    +
         /**
          * for wrap around ranges, which begin > end.
          * Sorting end ascending, so that token (begin, begin) will come after (begin, end]
with the same begin,
          * and (begin, end) won't be selected in the tailMap.
          */
    -    static final Comparator<Range<Token>> descendingComparatorForWrapAround
= new Comparator<Range<Token>>()
    -    {
    -        @Override
    -        public int compare(Range<Token> o1, Range<Token> o2)
    -        {
    -            int res = o2.left.compareTo(o1.left);
    -            if (res != 0)
    -                return res;
    -            return o1.right.compareTo(o2.right);
    -        }
    +    private static final Comparator<Range<Token>> descendingComparatorForWrapAround
= (o1, o2) -> {
    +        int res = o2.left.compareTo(o1.left);
    +        if (res != 0)
    +            return res;
    +        return o1.right.compareTo(o2.right);
         };
     
         public PendingRangeMaps()
         {
    -        this.ascendingMap = new TreeMap<Range<Token>, List<InetAddressAndPort>>(ascendingComparator);
    -        this.descendingMap = new TreeMap<Range<Token>, List<InetAddressAndPort>>(descendingComparator);
    -        this.ascendingMapForWrapAround = new TreeMap<Range<Token>, List<InetAddressAndPort>>(ascendingComparatorForWrapAround);
    -        this.descendingMapForWrapAround = new TreeMap<Range<Token>, List<InetAddressAndPort>>(descendingComparatorForWrapAround);
    +        this.ascendingMap = new TreeMap<>(ascendingComparator);
    +        this.descendingMap = new TreeMap<>(descendingComparator);
    +        this.ascendingMapForWrapAround = new TreeMap<>(ascendingComparatorForWrapAround);
    +        this.descendingMapForWrapAround = new TreeMap<>(descendingComparatorForWrapAround);
         }
     
         static final void addToMap(Range<Token> range,
    -                               InetAddressAndPort address,
    -                               NavigableMap<Range<Token>, List<InetAddressAndPort>>
ascendingMap,
    -                               NavigableMap<Range<Token>, List<InetAddressAndPort>>
descendingMap)
    +                               Replica replica,
    +                               NavigableMap<Range<Token>, List<Replica>>
ascendingMap,
    +                               NavigableMap<Range<Token>, List<Replica>>
descendingMap)
         {
    -        List<InetAddressAndPort> addresses = ascendingMap.get(range);
    -        if (addresses == null)
    +        List<Replica> replicas = ascendingMap.get(range);
    +        if (replicas == null)
             {
    -            addresses = new ArrayList<>(1);
    -            ascendingMap.put(range, addresses);
    -            descendingMap.put(range, addresses);
    +            replicas = new ArrayList<>(1);
    +            ascendingMap.put(range, replicas);
    +            descendingMap.put(range, replicas);
             }
    -        addresses.add(address);
    +        replicas.add(replica);
         }
     
    -    public void addPendingRange(Range<Token> range, InetAddressAndPort address)
    +    public void addPendingRange(Range<Token> range, Replica replica)
         {
             if (Range.isWrapAround(range.left, range.right))
             {
    -            addToMap(range, address, ascendingMapForWrapAround, descendingMapForWrapAround);
    +            addToMap(range, replica, ascendingMapForWrapAround, descendingMapForWrapAround);
             }
             else
             {
    -            addToMap(range, address, ascendingMap, descendingMap);
    +            addToMap(range, replica, ascendingMap, descendingMap);
             }
         }
     
    -    static final void addIntersections(Set<InetAddressAndPort> endpointsToAdd,
    -                                       NavigableMap<Range<Token>, List<InetAddressAndPort>>
smallerMap,
    -                                       NavigableMap<Range<Token>, List<InetAddressAndPort>>
biggerMap)
    +    static final void addIntersections(ReplicaSet replicasToAdd,
    +                                       NavigableMap<Range<Token>, List<Replica>>
smallerMap,
    +                                       NavigableMap<Range<Token>, List<Replica>>
biggerMap)
         {
             // find the intersection of two sets
             for (Range<Token> range : smallerMap.keySet())
             {
    -            List<InetAddressAndPort> addresses = biggerMap.get(range);
    -            if (addresses != null)
    +            List<Replica> replicas = biggerMap.get(range);
    +            if (replicas != null)
                 {
    -                endpointsToAdd.addAll(addresses);
    +                replicasToAdd.addAll(replicas);
                 }
             }
         }
     
    -    public Collection<InetAddressAndPort> pendingEndpointsFor(Token token)
    +    public ReplicaSet pendingEndpointsFor(Token token)
         {
    -        Set<InetAddressAndPort> endpoints = new HashSet<>();
    +        ReplicaSet replicas = new ReplicaSet();
     
    -        Range searchRange = new Range(token, token);
    +        Range<Token> searchRange = new Range<>(token, token);
     
             // search for non-wrap-around maps
    -        NavigableMap<Range<Token>, List<InetAddressAndPort>> ascendingTailMap
= ascendingMap.tailMap(searchRange, true);
    -        NavigableMap<Range<Token>, List<InetAddressAndPort>> descendingTailMap
= descendingMap.tailMap(searchRange, false);
    +        NavigableMap<Range<Token>, List<Replica>> ascendingTailMap
= ascendingMap.tailMap(searchRange, true);
    +        NavigableMap<Range<Token>, List<Replica>> descendingTailMap
= descendingMap.tailMap(searchRange, false);
     
             // add intersections of two maps
             if (ascendingTailMap.size() < descendingTailMap.size())
             {
    -            addIntersections(endpoints, ascendingTailMap, descendingTailMap);
    +            addIntersections(replicas, ascendingTailMap, descendingTailMap);
             }
             else
             {
    -            addIntersections(endpoints, descendingTailMap, ascendingTailMap);
    +            addIntersections(replicas, descendingTailMap, ascendingTailMap);
             }
     
             // search for wrap-around sets
             ascendingTailMap = ascendingMapForWrapAround.tailMap(searchRange, true);
             descendingTailMap = descendingMapForWrapAround.tailMap(searchRange, false);
     
             // add them since they are all necessary.
    -        for (Map.Entry<Range<Token>, List<InetAddressAndPort>> entry
: ascendingTailMap.entrySet())
    +        for (Map.Entry<Range<Token>, List<Replica>> entry : ascendingTailMap.entrySet())
             {
    -            endpoints.addAll(entry.getValue());
    +            replicas.addAll(entry.getValue());
             }
    -        for (Map.Entry<Range<Token>, List<InetAddressAndPort>> entry
: descendingTailMap.entrySet())
    +        for (Map.Entry<Range<Token>, List<Replica>> entry : descendingTailMap.entrySet())
             {
    -            endpoints.addAll(entry.getValue());
    +            replicas.addAll(entry.getValue());
             }
     
    -        return endpoints;
    +        return replicas;
         }
     
         public String printPendingRanges()
         {
             StringBuilder sb = new StringBuilder();
     
    -        for (Map.Entry<Range<Token>, List<InetAddressAndPort>> entry
: this)
    +        for (Map.Entry<Range<Token>, List<Replica>> entry : this)
             {
                 Range<Token> range = entry.getKey();
     
    -            for (InetAddressAndPort address : entry.getValue())
    +            for (Replica replica : entry.getValue())
                 {
    -                sb.append(address).append(':').append(range);
    +                sb.append(replica).append(':').append(range);
    --- End diff --
    
    What kind of compatibility issues are we creating here by changing the output? (And what
kind did I create with InetAddressAndPort?)
    
    Doesn't look like an issue to me since it's used in a trace log statement and TokenMetadata.toString().


---

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


Mime
View raw message