keith-turner commented on code in PR #5998:
URL: https://github.com/apache/accumulo/pull/5998#discussion_r2625097425
##########
core/src/test/java/org/apache/accumulo/core/iterators/system/MultiIteratorTest.java:
##########
@@ -422,4 +422,53 @@ public void test7() throws IOException {
mi.seek(r7, EMPTY_COL_FAMS, false);
assertFalse(mi.hasTop());
}
+
+ @Test
+ public void testDeepCopy() throws IOException {
+ // TEst setting an endKey
+ TreeMap<Key,Value> tm1 = new TreeMap<>();
+ newKeyValue(tm1, 0, 3, false, "1");
+ newKeyValue(tm1, 0, 2, false, "2");
+ newKeyValue(tm1, 0, 1, false, "3");
+ newKeyValue(tm1, 0, 0, false, "4");
+ newKeyValue(tm1, 1, 2, false, "5");
+ newKeyValue(tm1, 1, 1, false, "6");
+ newKeyValue(tm1, 1, 0, false, "7");
+ newKeyValue(tm1, 2, 1, false, "8");
+ newKeyValue(tm1, 2, 0, false, "9");
+
+ List<SortedKeyValueIterator<Key,Value>> skvil = new ArrayList<>(1);
+ skvil.add(new SortedMapIterator(tm1));
+
+ KeyExtent extent = new KeyExtent(TableId.of("tablename"), newRow(1),
newRow(0));
+
+ MultiIterator mi = new MultiIterator(skvil, extent);
+ MultiIterator miCopy = mi.deepCopy(null);
+
+ Range r1 = new Range((Text) null, (Text) null);
+ mi.seek(r1, EMPTY_COL_FAMS, false);
+ assertTrue(mi.hasTop());
+ assertEquals("5", mi.getTopValue().toString());
+ mi.next();
+ assertTrue(mi.hasTop());
+ assertEquals("6", mi.getTopValue().toString());
Review Comment:
Its good to test that deep copies are independent pointers. This can be done
by offsetting and interleaving the current code like the following start to
this. This show calling next() on one does not impact the other.
```suggestion
mi.seek(r1, EMPTY_COL_FAMS, false);
miCopy.seek(r1, EMPTY_COL_FAMS, false);
assertTrue(mi.hasTop());
assertEquals("5", mi.getTopValue().toString());
mi.next();
assertTrue(mi.hasTop());
assertEquals("6", mi.getTopValue().toString());
assertTrue(miCopy.hasTop());
assertEquals("5", miCopy.getTopValue().toString());
.
.
.
```
##########
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/MultiIterator.java:
##########
@@ -50,6 +51,7 @@ private MultiIterator(MultiIterator other,
IteratorEnvironment env) {
super(other.iters.size());
this.iters = new ArrayList<>();
this.fence = other.fence;
+ Collections.shuffle(other.iters);
Review Comment:
These shuffles will make seeks happen in different order. There is still
the case of initially opening the RFiles in FileManager that we could also
shuffle.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
