From 3a93a3c0f2e95ecf46624227d49b9cf74893092f Mon Sep 17 00:00:00 2001 From: DomGarguilo Date: Thu, 2 Feb 2023 13:58:06 -0500 Subject: [PATCH 1/4] WIP - fix CompactionSelecor test --- .../accumulo/proxy/its/SimpleProxyBase.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java index 7af98c0..4e60ffa 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -47,6 +47,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import java.util.stream.Stream; import org.apache.accumulo.cluster.ClusterUser; @@ -2216,6 +2217,7 @@ private int countFiles(String table) throws Exception { break; } } + client.closeScanner(scanner); return result; } @@ -2277,7 +2279,20 @@ public void testCompactionSelector() throws Exception { client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); // SelectHalfSelector should lead to half the files being compacted - Wait.waitFor(() -> countFiles(tableName) == (expectedFileCount / 2)); + final int halfOfOriginalFileCount = expectedFileCount / 2; + + // a supplier is needed for the message so the file count will be computed lazily + Supplier message = () -> { + int fileCount; + try { + fileCount = countFiles(tableName); + } catch (Exception e) { + throw new RuntimeException(e); + } + return "Expected to find " + halfOfOriginalFileCount + " files but found " + fileCount; + }; + + assertTrue(Wait.waitFor(() -> countFiles(tableName) == halfOfOriginalFileCount), message); } @Test From 8dadc39df21c8bd6f09333e8d1bd87857137606d Mon Sep 17 00:00:00 2001 From: DomGarguilo Date: Mon, 6 Feb 2023 15:30:41 -0500 Subject: [PATCH 2/4] Use TooManyDeletesSelector --- .../proxy/its/SelectHalfSelector.java | 45 --------------- .../accumulo/proxy/its/SimpleProxyBase.java | 56 +++++++++++-------- 2 files changed, 33 insertions(+), 68 deletions(-) delete mode 100644 src/test/java/org/apache/accumulo/proxy/its/SelectHalfSelector.java diff --git a/src/test/java/org/apache/accumulo/proxy/its/SelectHalfSelector.java b/src/test/java/org/apache/accumulo/proxy/its/SelectHalfSelector.java deleted file mode 100644 index 758c618..0000000 --- a/src/test/java/org/apache/accumulo/proxy/its/SelectHalfSelector.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.accumulo.proxy.its; - -import java.util.List; -import java.util.stream.Collectors; - -import org.apache.accumulo.core.client.admin.compaction.CompactableFile; -import org.apache.accumulo.core.client.admin.compaction.CompactionSelector; - -/** - * Select half of the files to compact - */ -public class SelectHalfSelector implements CompactionSelector { - - @Override - public void init(InitParameters iparams) {} - - @Override - public Selection select(SelectionParameters sparams) { - final var totalFiles = sparams.getAvailableFiles(); - - final int halfOfFileCount = totalFiles.size() / 2; - - final List toCompact = - totalFiles.stream().limit(halfOfFileCount).collect(Collectors.toList()); - - return new Selection(toCompact); - } - -} diff --git a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java index 4e60ffa..13d0e95 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -47,14 +47,15 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; import java.util.stream.Stream; import org.apache.accumulo.cluster.ClusterUser; import org.apache.accumulo.core.client.Accumulo; import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.admin.compaction.TooManyDeletesSelector; import org.apache.accumulo.core.client.security.tokens.KerberosToken; import org.apache.accumulo.core.client.security.tokens.PasswordToken; +import org.apache.accumulo.core.client.summary.summarizers.DeletesSummarizer; import org.apache.accumulo.core.clientImpl.ClientInfo; import org.apache.accumulo.core.clientImpl.Namespace; import org.apache.accumulo.core.conf.DefaultConfiguration; @@ -2262,37 +2263,46 @@ public void testGetRowRange() throws Exception { @Test public void testCompactionSelector() throws Exception { - String[] data = "A B C D E F G H I J K L M N O P Q R S T U V W X Y Z".split(" "); - final int expectedFileCount = data.length; + client.setTableProperty(sharedSecret, tableName, Property.TABLE_SUMMARIZER_PREFIX + "s1", + DeletesSummarizer.class.getName()); - for (String datum : data) { - client.addSplits(sharedSecret, tableName, Set.of(s2bb(datum))); - client.updateAndFlush(sharedSecret, tableName, mutation(datum, "cf", "cq", datum)); - client.flushTable(sharedSecret, tableName, null, null, true); + Map> mutation = new HashMap<>(); + + for (int i = 1; i < 2000; i++) { + String row = String.format("%09d", i); + ColumnUpdate columnUpdate = newColUpdate("cf", "cq", "v" + i); + mutation.put(s2bb(row), List.of(columnUpdate)); } - assertEquals(expectedFileCount, countFiles(tableName), "Unexpected file count"); + client.updateAndFlush(sharedSecret, tableName, mutation); + client.flushTable(sharedSecret, tableName, null, null, true); + + mutation.clear(); + + for (int i = 1; i < 1000; i++) { + String row = String.format("%09d", i); + ColumnUpdate columnUpdate = newColUpdate("cf", "cq", "v" + i); + columnUpdate.setDeleteCell(true); + mutation.put(s2bb(row), List.of(columnUpdate)); + } + + client.updateAndFlush(sharedSecret, tableName, mutation); + client.flushTable(sharedSecret, tableName, null, null, true); + + assertEquals(2, countFiles(tableName)); - String selectorClassname = SelectHalfSelector.class.getName(); - PluginConfig selector = new PluginConfig(selectorClassname, Map.of()); + String selectorClassname = TooManyDeletesSelector.class.getName(); + PluginConfig selector = new PluginConfig(selectorClassname, Map.of("threshold", ".99")); client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); - // SelectHalfSelector should lead to half the files being compacted - final int halfOfOriginalFileCount = expectedFileCount / 2; + assertEquals(2, countFiles(tableName)); - // a supplier is needed for the message so the file count will be computed lazily - Supplier message = () -> { - int fileCount; - try { - fileCount = countFiles(tableName); - } catch (Exception e) { - throw new RuntimeException(e); - } - return "Expected to find " + halfOfOriginalFileCount + " files but found " + fileCount; - }; + selector = new PluginConfig(selectorClassname, Map.of("threshold", ".40")); - assertTrue(Wait.waitFor(() -> countFiles(tableName) == halfOfOriginalFileCount), message); + client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); + + assertEquals(1, countFiles(tableName)); } @Test From a8b00543c7720e32b291f7a9f4cc8e82087f7b14 Mon Sep 17 00:00:00 2001 From: DomGarguilo Date: Tue, 7 Feb 2023 10:05:48 -0500 Subject: [PATCH 3/4] Encorporate more test cases --- .../accumulo/proxy/its/SimpleProxyBase.java | 76 ++++++++++++++----- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java index 13d0e95..f415692 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -2260,49 +2260,83 @@ public void testGetRowRange() throws Exception { assertEquals(range.stop.timestamp, range.stop.timestamp); } - @Test - public void testCompactionSelector() throws Exception { - - client.setTableProperty(sharedSecret, tableName, Property.TABLE_SUMMARIZER_PREFIX + "s1", - DeletesSummarizer.class.getName()); - + private void addFile(String tableName, int startRow, int endRow, boolean delete) + throws TException { Map> mutation = new HashMap<>(); - for (int i = 1; i < 2000; i++) { + for (int i = startRow; i < endRow; i++) { String row = String.format("%09d", i); ColumnUpdate columnUpdate = newColUpdate("cf", "cq", "v" + i); + columnUpdate.setDeleteCell(delete); mutation.put(s2bb(row), List.of(columnUpdate)); } client.updateAndFlush(sharedSecret, tableName, mutation); client.flushTable(sharedSecret, tableName, null, null, true); + } + + @Test + public void testCompactionSelector() throws Exception { - mutation.clear(); + // delete table so new tables won't have the same name + client.deleteTable(sharedSecret, tableName); - for (int i = 1; i < 1000; i++) { - String row = String.format("%09d", i); - ColumnUpdate columnUpdate = newColUpdate("cf", "cq", "v" + i); - columnUpdate.setDeleteCell(true); - mutation.put(s2bb(row), List.of(columnUpdate)); + final String[] tableNames = getUniqueNameArray(3); + + // for each table, set the summarizer property needed for TooManyDeletesSelector + final String summarizerKey = Property.TABLE_SUMMARIZER_PREFIX + "s1"; + final String summarizerClassName = DeletesSummarizer.class.getName(); + + for (String tableName : tableNames) { + client.createTable(sharedSecret, tableName, true, TimeType.MILLIS); + client.setTableProperty(sharedSecret, tableName, summarizerKey, summarizerClassName); } - client.updateAndFlush(sharedSecret, tableName, mutation); - client.flushTable(sharedSecret, tableName, null, null, true); + // add files to each table + + addFile(tableNames[0], 1, 1000, false); + addFile(tableNames[0], 1, 1000, true); + + addFile(tableNames[1], 1, 1000, false); + addFile(tableNames[1], 1000, 2000, false); + + addFile(tableNames[2], 1, 2000, false); + addFile(tableNames[2], 1, 1000, true); + + final String messagePrefix = "Unexpected file count on table "; + + for (String tableName : tableNames) { + assertEquals(2, countFiles(tableName), messagePrefix + tableName); + } - assertEquals(2, countFiles(tableName)); + // compact the tables and check for expected file counts - String selectorClassname = TooManyDeletesSelector.class.getName(); + final String selectorClassname = TooManyDeletesSelector.class.getName(); PluginConfig selector = new PluginConfig(selectorClassname, Map.of("threshold", ".99")); - client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); + for (String tableName : tableNames) { + client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); + } + + assertEquals(0, countFiles(tableNames[0]), messagePrefix + tableNames[0]); + assertEquals(2, countFiles(tableNames[1]), messagePrefix + tableNames[1]); + assertEquals(2, countFiles(tableNames[2]), messagePrefix + tableNames[2]); - assertEquals(2, countFiles(tableName)); + // create a selector with different properties then compact and check file counts selector = new PluginConfig(selectorClassname, Map.of("threshold", ".40")); - client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); + for (String tableName : tableNames) { + client.compactTable(sharedSecret, tableName, null, null, null, true, true, selector, null); + } - assertEquals(1, countFiles(tableName)); + assertEquals(0, countFiles(tableNames[0]), messagePrefix + tableNames[0]); + assertEquals(2, countFiles(tableNames[1]), messagePrefix + tableNames[1]); + assertEquals(1, countFiles(tableNames[2]), messagePrefix + tableNames[2]); + + client.compactTable(sharedSecret, tableNames[1], null, null, null, true, true, null, null); + + assertEquals(1, countFiles(tableNames[1]), messagePrefix + tableNames[2]); } @Test From fe6ceca26df8e5ac1ec88b0ae03074bc33a4e836 Mon Sep 17 00:00:00 2001 From: DomGarguilo Date: Tue, 7 Feb 2023 10:25:16 -0500 Subject: [PATCH 4/4] Add comment describing the test --- .../java/org/apache/accumulo/proxy/its/SimpleProxyBase.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java index f415692..a42ef8e 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -2275,6 +2275,10 @@ private void addFile(String tableName, int startRow, int endRow, boolean delete) client.flushTable(sharedSecret, tableName, null, null, true); } + /** + * Test to make sure we can specify a Selector from the proxy client, and it will take effect when + * compactions occur + */ @Test public void testCompactionSelector() throws Exception {