kevinrr888 commented on code in PR #6075:
URL: https://github.com/apache/accumulo/pull/6075#discussion_r2729254412
##########
test/src/main/java/org/apache/accumulo/test/functional/CompactionIT.java:
##########
@@ -988,6 +989,54 @@ public void testMigrationCancelCompaction() throws
Exception {
}
}
+ @Test
+ public void testIteratorOrder() throws Exception {
+ String[] names = getUniqueNames(2);
+ try (AccumuloClient c =
Accumulo.newClient().from(getClientProps()).build()) {
+
+ // create a table with minor compaction iterators configured to ensure
those iterators are
+ // applied in the correct order
+ NewTableConfiguration ntc = new NewTableConfiguration()
+ .attachIterator(AppendingIterator.configure(50, "x"),
EnumSet.of(IteratorScope.minc))
+ .attachIterator(AppendingIterator.configure(100, "a"),
EnumSet.of(IteratorScope.minc));
+ c.tableOperations().create(names[0], ntc);
+
+ // create a table with major compaction iterators configured to ensure
those iterators are
+ // applied in the correct order
+ NewTableConfiguration ntc2 = new NewTableConfiguration()
+ .attachIterator(AppendingIterator.configure(50, "x"),
EnumSet.of(IteratorScope.majc))
+ .attachIterator(AppendingIterator.configure(100, "a"),
EnumSet.of(IteratorScope.majc));
+ c.tableOperations().create(names[1], ntc2);
+
+ try (var writer = c.createBatchWriter(names[0]);
+ var writer2 = c.createBatchWriter(names[1])) {
+ Mutation m = new Mutation("r1");
+ m.put("", "", "base:");
+ writer.addMutation(m);
+ writer2.addMutation(m);
+ }
+
+ try (var mincScanner = c.createScanner(names[0]);
+ var majcScanner = c.createScanner(names[1])) {
+ // iterators should not be applied yet
+ assertEquals("base:",
mincScanner.iterator().next().getValue().toString());
+ assertEquals("base:",
majcScanner.iterator().next().getValue().toString());
+
+ c.tableOperations().flush(names[0], null, null, true);
+ assertEquals("base:xa",
mincScanner.iterator().next().getValue().toString());
+ assertEquals("base:",
majcScanner.iterator().next().getValue().toString());
+
+ List<IteratorSetting> iters = List.of(AppendingIterator.configure(70,
"m"),
+ AppendingIterator.configure(50, "b"),
AppendingIterator.configure(100, "c"));
+ c.tableOperations().compact(names[1],
+ new
CompactionConfig().setWait(true).setFlush(true).setIterators(iters));
Review Comment:
A few comments:
1) This made me realize I did not consider `CompactionConfig` in #6040, so
even with 6040, this call will not log a warning (should log a warning in 2.1
since we are setting a majc iter with the same priority as a majc iter set on
the table). To avoid adding more to that PR, should probably open a follow on
issue to address this after 6040 is merged.
2) I think all of the testing added in this PR will need to be removed in
main when #6040 is merged up to main. This is because adding an iterator of the
same priority will cause an exception in main. As mentioned above, there's the
`CompactionConfig` caveat for now, but this testing should still be removed
with the merging of 6040 into main. The follow on issue will address making
`CompactionConfig` throw in the case of attempting to add an iter of existing
priority (or name) and should add testing to `IteratorConflictsIT` to confirm
it does throw.
3) I can keep the changes made to `ITER_INFO_COMPARATOR` in this PR in main
for consistency
These are comments for dicussion/notes-to-self when I merge 6040, nothing to
change in this PR
##########
test/src/main/java/org/apache/accumulo/test/functional/CompactionIT.java:
##########
@@ -988,6 +989,54 @@ public void testMigrationCancelCompaction() throws
Exception {
}
}
+ @Test
+ public void testIteratorOrder() throws Exception {
+ String[] names = getUniqueNames(2);
+ try (AccumuloClient c =
Accumulo.newClient().from(getClientProps()).build()) {
+
+ // create a table with minor compaction iterators configured to ensure
those iterators are
+ // applied in the correct order
+ NewTableConfiguration ntc = new NewTableConfiguration()
+ .attachIterator(AppendingIterator.configure(50, "x"),
EnumSet.of(IteratorScope.minc))
+ .attachIterator(AppendingIterator.configure(100, "a"),
EnumSet.of(IteratorScope.minc));
+ c.tableOperations().create(names[0], ntc);
+
+ // create a table with major compaction iterators configured to ensure
those iterators are
+ // applied in the correct order
+ NewTableConfiguration ntc2 = new NewTableConfiguration()
+ .attachIterator(AppendingIterator.configure(50, "x"),
EnumSet.of(IteratorScope.majc))
+ .attachIterator(AppendingIterator.configure(100, "a"),
EnumSet.of(IteratorScope.majc));
+ c.tableOperations().create(names[1], ntc2);
+
+ try (var writer = c.createBatchWriter(names[0]);
+ var writer2 = c.createBatchWriter(names[1])) {
+ Mutation m = new Mutation("r1");
+ m.put("", "", "base:");
+ writer.addMutation(m);
+ writer2.addMutation(m);
+ }
+
+ try (var mincScanner = c.createScanner(names[0]);
+ var majcScanner = c.createScanner(names[1])) {
+ // iterators should not be applied yet
+ assertEquals("base:",
mincScanner.iterator().next().getValue().toString());
+ assertEquals("base:",
majcScanner.iterator().next().getValue().toString());
+
+ c.tableOperations().flush(names[0], null, null, true);
+ assertEquals("base:xa",
mincScanner.iterator().next().getValue().toString());
+ assertEquals("base:",
majcScanner.iterator().next().getValue().toString());
+
+ List<IteratorSetting> iters = List.of(AppendingIterator.configure(70,
"m"),
+ AppendingIterator.configure(50, "b"),
AppendingIterator.configure(100, "c"));
+ c.tableOperations().compact(names[1],
+ new
CompactionConfig().setWait(true).setFlush(true).setIterators(iters));
+ assertEquals("base:xa",
mincScanner.iterator().next().getValue().toString());
+ assertEquals("base:bxmac",
majcScanner.iterator().next().getValue().toString());
Review Comment:
A comment to note that (50, "b") and (100, "c") share the same priority as
other iterators would be helpful
--
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]
