You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "Yukei7 (via GitHub)" <gi...@apache.org> on 2024/12/20 13:06:44 UTC

[PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Yukei7 opened a new pull request, #3754:
URL: https://github.com/apache/cassandra/pull/3754

   Thanks for sending a pull request! Here are some tips if you're new here:
    
    * Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
    * Be sure to keep the PR description updated to reflect all changes.
    * Write your PR title to summarize what this PR proposes.
    * If possible, provide a concise example to reproduce the issue for a faster review.
    * Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
    * If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
    
   Commit messages should follow the following format:
   
   ```
   IntervalTree should support copyAndReplace for checkpoint when ranges are unchanged
   
   patch by Yuqi Yan; reviewed by TBD for CASSANDRA-20158
   
   ```
   
   The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/CASSANDRA-20158)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1945686549


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,19 @@ public boolean apply(T t)
             }
         };
     }
+
+    private static int getSSTablesHash(Iterable<SSTableReader> readers) {
+        int hashSum = 0;

Review Comment:
   A 32-bit non-cryptographic hash isn't strong enough here. This could realistically collide.
   
   It would really be better to arrange it so that you can compare them directly by say making sure the incoming iteration order is on something like sstable id.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1984159806


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,40 @@ public boolean apply(T t)
             }
         };
     }
+
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    {
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor.id). The view is per cfs so id will be unique
+        Comparator<SSTableReader> comp = Comparator.comparing((SSTableReader s) -> s.first)

Review Comment:
   This does replacement via the `first` and `last` so it won't help with early open` which is replacing an sstable with a different `first` and `last`.
   
   The only thing we can really properly replace by is the ID which can be found in the tree somewhere between the `first` and `last` of the replacement. I think in practice the replacement should always share the same `first` since the `first` can't change beyond going from `null` to not `null`.
   
   Incidentally does this fail `scrub_test.py`? It turns out offline tools load `sstables` without initializing the first and last key. At least they do in older versions.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1946786489


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -372,8 +372,45 @@ private static int getSSTablesHash(Iterable<SSTableReader> readers) {
         return hashSum;
     }
 
-    private static boolean isRangesUnchanged(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
     {
-        return getSSTablesHash(remove) == getSSTablesHash(add);
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor)
+        Comparator<SSTableReader> comp = (o1, o2) -> {
+            if (o1.first.compareTo(o2.first) == 0)
+                if (o1.last.compareTo(o2.last) == 0)
+                    return Integer.compare(o1.descriptor.hashCode(), o2.descriptor.hashCode());

Review Comment:
   I am little confused, this uses `hashCode` still?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "Yukei7 (via GitHub)" <gi...@apache.org>.
Yukei7 commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1945742217


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,19 @@ public boolean apply(T t)
             }
         };
     }
+
+    private static int getSSTablesHash(Iterable<SSTableReader> readers) {
+        int hashSum = 0;

Review Comment:
   updated this method a bit - instead of calculating the hash, it's probably better to find a 1-1 mapping from `toAdd` and `toRemove`. If their range is the same and such 1-1 mapping exists, we can simply replaced those.
   let me know if this works or you have better idea here 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "Yukei7 (via GitHub)" <gi...@apache.org>.
Yukei7 commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1947116459


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -372,8 +372,45 @@ private static int getSSTablesHash(Iterable<SSTableReader> readers) {
         return hashSum;
     }
 
-    private static boolean isRangesUnchanged(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
     {
-        return getSSTablesHash(remove) == getSSTablesHash(add);
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor)
+        Comparator<SSTableReader> comp = (o1, o2) -> {
+            if (o1.first.compareTo(o2.first) == 0)
+                if (o1.last.compareTo(o2.last) == 0)
+                    return Integer.compare(o1.descriptor.hashCode(), o2.descriptor.hashCode());

Review Comment:
   oh I see.. I was under impression that `equals` method for `descriptor` was with `hashCode`.
   
   I update it to use the `idComparator` to sort by descriptor.id. These are within the same cfs so id will be unique



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1984159444


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,40 @@ public boolean apply(T t)
             }
         };
     }
+
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    {
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor.id). The view is per cfs so id will be unique
+        Comparator<SSTableReader> comp = Comparator.comparing((SSTableReader s) -> s.first)
+                                                   .thenComparing(s -> s.last)
+                                                   .thenComparing(SSTableReader.idComparator);
+        toRemoves.sort(comp);
+        toAdds.sort(comp);
+
+        Map<SSTableReader, SSTableReader> replacementMap = new HashMap<>();
+        // toAdd and toRemove have the same size
+        for (int i = 0; i < toAdds.size(); i++)
+        {
+            SSTableReader toRemove = toRemoves.get(i);
+            SSTableReader toAdd = toAdds.get(i);
+            // optimization: here we don't check the descriptor. If we're able to match those to be removed with those
+            // to be added, we ensure that the pairs have the same (first, last) range
+            if (toRemove.first.equals(toAdd.first) && toRemove.last.equals(toAdd.last))

Review Comment:
   How is this optimization safe? Multiple sstables can share the same first and last.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg closed pull request #3754: IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158)
URL: https://github.com/apache/cassandra/pull/3754


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1984160256


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,40 @@ public boolean apply(T t)
             }
         };
     }
+
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    {
+        List<SSTableReader> toAdds = new ArrayList<>();

Review Comment:
   The copy and sorting of `toAdds` and `toRemove` could be done outside the lock/CAS loop since it doesn't depend on the contents of the `View`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "Yukei7 (via GitHub)" <gi...@apache.org>.
Yukei7 commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1984281912


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,40 @@ public boolean apply(T t)
             }
         };
     }
+
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    {
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor.id). The view is per cfs so id will be unique
+        Comparator<SSTableReader> comp = Comparator.comparing((SSTableReader s) -> s.first)
+                                                   .thenComparing(s -> s.last)
+                                                   .thenComparing(SSTableReader.idComparator);
+        toRemoves.sort(comp);
+        toAdds.sort(comp);
+
+        Map<SSTableReader, SSTableReader> replacementMap = new HashMap<>();
+        // toAdd and toRemove have the same size
+        for (int i = 0; i < toAdds.size(); i++)
+        {
+            SSTableReader toRemove = toRemoves.get(i);
+            SSTableReader toAdd = toAdds.get(i);
+            // optimization: here we don't check the descriptor. If we're able to match those to be removed with those
+            // to be added, we ensure that the pairs have the same (first, last) range
+            if (toRemove.first.equals(toAdd.first) && toRemove.last.equals(toAdd.last))

Review Comment:
   IntervalTree only search with `searchInterval.min` and `searchInterval.max`, all sstables intersect with this will be returned. It's safe if we properly construct this map to ensure that toAdd and toRemove is 1-to-1 mapping with same [first,last], and in the replace method we do check on `toRemove` equals to the interval in the tree.
   
   e.g.
   Say T1, T2, T3 all share [1,4]
   We replace T2 with T2' also share with [1,4]
   toAdd = T2', toRemove=T2
   The map here will be {T2: T2'}
   
   When actually figuring the intervals to do replacement, we do check: `node.intersectsLeft.get(i).equals(intervalToRemove)`
   
   That we start checking from T1, but `!T1.equals(T2)` and we continue to the correct place T2.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "Yukei7 (via GitHub)" <gi...@apache.org>.
Yukei7 commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1984281912


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,40 @@ public boolean apply(T t)
             }
         };
     }
+
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    {
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor.id). The view is per cfs so id will be unique
+        Comparator<SSTableReader> comp = Comparator.comparing((SSTableReader s) -> s.first)
+                                                   .thenComparing(s -> s.last)
+                                                   .thenComparing(SSTableReader.idComparator);
+        toRemoves.sort(comp);
+        toAdds.sort(comp);
+
+        Map<SSTableReader, SSTableReader> replacementMap = new HashMap<>();
+        // toAdd and toRemove have the same size
+        for (int i = 0; i < toAdds.size(); i++)
+        {
+            SSTableReader toRemove = toRemoves.get(i);
+            SSTableReader toAdd = toAdds.get(i);
+            // optimization: here we don't check the descriptor. If we're able to match those to be removed with those
+            // to be added, we ensure that the pairs have the same (first, last) range
+            if (toRemove.first.equals(toAdd.first) && toRemove.last.equals(toAdd.last))

Review Comment:
   IntervalTree only search with `searchInterval.min` and `searchInterval.max`, all sstables intersect with this will be returned. It's safe if we properly construct this map to ensure that toAdd and toRemove is 1-to-1 mapping with same [first,last], and in the replace method we do check on `toRemove` equals to the interval in the tree.
   
   e.g.
   Say T1, T2, T3 all share [1,4]
   We replace T2 with T2' also share with [1,4]
   toAdd = T2', toRemove=T2
   The map here will be {T2: T2'}
   
   When actually figuring the intervals to do replacement, we do check: `node.intersectsLeft.get(i).equals(intervalToRemove)`
   
   `equals` for `Interval` do check on `data` field (In case of `SSTableReader` this will ensure the `descriptor` matches):
   
   ```
   return Objects.equal(min, that.min) && Objects.equal(max, that.max) && Objects.equal(data, that.data);
   ```
   
   That we start checking from T1, but `!T1.equals(T2)` and we continue to the correct place T2.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] IntervalTree should support copyAndReplace when ranges are unchanged (CASSANDRA-20158) [cassandra]

Posted by "Yukei7 (via GitHub)" <gi...@apache.org>.
Yukei7 commented on code in PR #3754:
URL: https://github.com/apache/cassandra/pull/3754#discussion_r1984281912


##########
src/java/org/apache/cassandra/db/lifecycle/View.java:
##########
@@ -353,4 +361,40 @@ public boolean apply(T t)
             }
         };
     }
+
+    // Match the SSTableReaders from the existing ones to the new one to be added (with same ranges)
+    // Returns the map of toRemove <-> toAdd. Return empty map if such 1-1 replacement doesn't exist
+    private static Map<SSTableReader, SSTableReader> getReplacementMap(final Set<SSTableReader> remove, final Iterable<SSTableReader> add)
+    {
+        List<SSTableReader> toAdds = new ArrayList<>();
+        for (SSTableReader s : add)
+            toAdds.add(s);
+
+        if (remove.size() != toAdds.size())
+            return Collections.emptyMap();
+
+        List<SSTableReader> toRemoves = new ArrayList<>(remove);
+        // sort the SSTableReader list by (first, last, descriptor.id). The view is per cfs so id will be unique
+        Comparator<SSTableReader> comp = Comparator.comparing((SSTableReader s) -> s.first)
+                                                   .thenComparing(s -> s.last)
+                                                   .thenComparing(SSTableReader.idComparator);
+        toRemoves.sort(comp);
+        toAdds.sort(comp);
+
+        Map<SSTableReader, SSTableReader> replacementMap = new HashMap<>();
+        // toAdd and toRemove have the same size
+        for (int i = 0; i < toAdds.size(); i++)
+        {
+            SSTableReader toRemove = toRemoves.get(i);
+            SSTableReader toAdd = toAdds.get(i);
+            // optimization: here we don't check the descriptor. If we're able to match those to be removed with those
+            // to be added, we ensure that the pairs have the same (first, last) range
+            if (toRemove.first.equals(toAdd.first) && toRemove.last.equals(toAdd.last))

Review Comment:
   IntervalTree only search with `searchInterval.min` and `searchInterval.max`, all sstables intersect with this will be returned. It's safe if we properly construct this map to ensure that toAdd and toRemove is 1-to-1 mapping with same [first,last], and in the replace method we do check on `toRemove` equals to the interval in the tree.
   
   e.g.
   Say T1, T2, T3 all share [1,4]
   We replace T2 with T2' also share with [1,4]
   toAdd = T2', toRemove=T2
   The map here will be {T2: T2'}
   
   When actually figuring the intervals to do replacement, we do check: `node.intersectsLeft.get(i).equals(intervalToRemove)`
   
   `equals` for `Interval` do check on `data` field:
   
   ```
   return Objects.equal(min, that.min) && Objects.equal(max, that.max) && Objects.equal(data, that.data);
   ```
   
   That we start checking from T1, but `!T1.equals(T2)` and we continue to the correct place T2.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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