From 109bb6b4d72f33e4638cd64ad720d7912e233a9d Mon Sep 17 00:00:00 2001 From: Robert Kurc Date: Fri, 11 Jul 2025 00:56:18 +0000 Subject: [PATCH 1/6] Added a test in SimpleProxyBase.java that compacts a table, and loads the metadata for each tablet. Does not yet do anything with the tablet metadata. --- .../accumulo/proxy/its/SimpleProxyBase.java | 35 +++++++++++++++++++ 1 file changed, 35 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 5435135..2ea49ff 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -52,11 +52,13 @@ import org.apache.accumulo.core.client.Accumulo; import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.admin.compaction.CompressionConfigurer; import org.apache.accumulo.core.client.admin.compaction.TooManyDeletesSelector; import org.apache.accumulo.core.client.summary.summarizers.DeletesSummarizer; import org.apache.accumulo.core.clientImpl.Namespace; import org.apache.accumulo.core.conf.DefaultConfiguration; import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.file.FileOperations; import org.apache.accumulo.core.file.FileSKVWriter; @@ -66,6 +68,7 @@ import org.apache.accumulo.core.iterators.user.SummingCombiner; import org.apache.accumulo.core.iterators.user.VersioningIterator; import org.apache.accumulo.core.metadata.MetadataTable; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.security.Authorizations; import org.apache.accumulo.core.spi.crypto.NoCryptoServiceFactory; import org.apache.accumulo.core.util.ByteBufferUtil; @@ -114,6 +117,7 @@ import org.apache.accumulo.proxy.thrift.UnknownScanner; import org.apache.accumulo.proxy.thrift.UnknownWriter; import org.apache.accumulo.proxy.thrift.WriterOptions; +import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.util.PortUtils; import org.apache.accumulo.test.constraints.MaxMutationSize; import org.apache.accumulo.test.constraints.NumericValueConstraint; @@ -2315,6 +2319,37 @@ public void testCompactionSelector() throws Exception { assertEquals(1, countFiles(tableNames[1]), messagePrefix + tableNames[2]); } + /** + * Testing the functionality for the CompactionConfigurer + */ + @Test + public void testCompactionConfigurer() throws Exception { + // Create a table, and give it some data to be compacted + client.createTable(sharedSecret, tableName, true, TimeType.MILLIS); + addFile(tableName, 1, 1000, false); + + // Create a PluginConfig for the Configurer, then compact the tables to see their output + PluginConfig configurer = new PluginConfig(CompressionConfigurer.class.getName(), Map.of( + CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, "1", + CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); + + client.compactTable(sharedSecret, tableName, null, null, null, true, true, null, configurer); + assertCompactionMetadata(getCluster().getServerContext(), tableName); + } + + /** + * Asserts true if Compaction Metadata was found + */ + private void assertCompactionMetadata(ServerContext ctx, String tableName){ + var tableId = TableId.of(ctx.tableOperations().tableIdMap().get(tableName)); + try (var tabletsMetadata = ctx.getAmple().readTablets().forTable(tableId).build()) { + for (TabletMetadata tablet : tabletsMetadata) { + // TODO: Make this actually look for compaction metadata + assertEquals(tablet, tablet); + } + } + } + @Test public void namespaceOperations() throws Exception { // default namespace and accumulo namespace From 8816238a8bc852e393c0aee25e6193f51b09f134 Mon Sep 17 00:00:00 2001 From: Robert Kurc Date: Fri, 11 Jul 2025 17:00:54 +0000 Subject: [PATCH 2/6] Added changes to allow the test to pass without error. Test still does not test anything yet. --- .../apache/accumulo/proxy/its/SimpleProxyBase.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 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 2ea49ff..7bab885 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -2324,15 +2324,17 @@ public void testCompactionSelector() throws Exception { */ @Test public void testCompactionConfigurer() throws Exception { + // Delete the table to start fresh + client.deleteTable(sharedSecret, tableName); // Create a table, and give it some data to be compacted client.createTable(sharedSecret, tableName, true, TimeType.MILLIS); addFile(tableName, 1, 1000, false); // Create a PluginConfig for the Configurer, then compact the tables to see their output - PluginConfig configurer = new PluginConfig(CompressionConfigurer.class.getName(), Map.of( - CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, "1", - CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); - + PluginConfig configurer = new PluginConfig(CompressionConfigurer.class.getName(), + Map.of(CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, "1", + CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); + client.compactTable(sharedSecret, tableName, null, null, null, true, true, null, configurer); assertCompactionMetadata(getCluster().getServerContext(), tableName); } @@ -2340,7 +2342,7 @@ public void testCompactionConfigurer() throws Exception { /** * Asserts true if Compaction Metadata was found */ - private void assertCompactionMetadata(ServerContext ctx, String tableName){ + private void assertCompactionMetadata(ServerContext ctx, String tableName) { var tableId = TableId.of(ctx.tableOperations().tableIdMap().get(tableName)); try (var tabletsMetadata = ctx.getAmple().readTablets().forTable(tableId).build()) { for (TabletMetadata tablet : tabletsMetadata) { From 1bb4ed87b0f751aa063d58e16514efea75d957ac Mon Sep 17 00:00:00 2001 From: Robert Kurc Date: Sun, 13 Jul 2025 17:37:12 +0000 Subject: [PATCH 3/6] Test successfully works. However, the code coverage has yet to be checked. --- .../accumulo/proxy/its/SimpleProxyBase.java | 87 ++++++++++++++----- 1 file changed, 64 insertions(+), 23 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 7bab885..9437f56 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -31,6 +31,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStreamReader; +import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.URI; import java.nio.ByteBuffer; @@ -68,7 +69,6 @@ import org.apache.accumulo.core.iterators.user.SummingCombiner; import org.apache.accumulo.core.iterators.user.VersioningIterator; import org.apache.accumulo.core.metadata.MetadataTable; -import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.security.Authorizations; import org.apache.accumulo.core.spi.crypto.NoCryptoServiceFactory; import org.apache.accumulo.core.util.ByteBufferUtil; @@ -2320,36 +2320,77 @@ public void testCompactionSelector() throws Exception { } /** - * Testing the functionality for the CompactionConfigurer + * Retrieves the collective size of all the files in a table. + */ + private long getFileSizes(ServerContext ctx, String tableName) { + TableId tableId = TableId.of(ctx.tableOperations().tableIdMap().get(tableName)); + var tabletsMetadata = ctx.getAmple().readTablets().forTable(tableId).build(); + return tabletsMetadata.stream().flatMap(tm -> tm.getFiles().stream()).mapToLong(stf -> { + try { + return FileSystem.getLocal(new Configuration()).getFileStatus(stf.getPath()).getLen(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }).sum(); + } + + /** + * Testing the functionality for the CompactionConfigurer by testing an implementation of it. The + * implementation being tested is the CompressionConfigurer. */ @Test public void testCompactionConfigurer() throws Exception { // Delete the table to start fresh client.deleteTable(sharedSecret, tableName); - // Create a table, and give it some data to be compacted - client.createTable(sharedSecret, tableName, true, TimeType.MILLIS); - addFile(tableName, 1, 1000, false); - - // Create a PluginConfig for the Configurer, then compact the tables to see their output - PluginConfig configurer = new PluginConfig(CompressionConfigurer.class.getName(), - Map.of(CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, "1", - CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); + // Create two tables + final String[] tableNames = getUniqueNameArray(2); + for (String tableName : tableNames) { + client.createTable(sharedSecret, tableName, true, TimeType.MILLIS); + client.setTableProperty(sharedSecret, tableName, "table.file.compress.type", "none"); + } - client.compactTable(sharedSecret, tableName, null, null, null, true, true, null, configurer); - assertCompactionMetadata(getCluster().getServerContext(), tableName); - } + // Create data to add to the tables + Map> mutation = new HashMap<>(); + byte[] data = new byte[100000]; + Arrays.fill(data, (byte) 65); + for (int i = 0; i < 10; i++) { + String row = String.format("%09d", i); + ColumnUpdate columnUpdate = new ColumnUpdate(s2bb("big"), s2bb("files")); + columnUpdate.setDeleteCell(false); + columnUpdate.setValue(data); + mutation.put(s2bb(row), List.of(columnUpdate)); + } + for (String tableName : tableNames) { + client.updateAndFlush(sharedSecret, tableName, mutation); + client.flushTable(sharedSecret, tableName, null, null, true); + } - /** - * Asserts true if Compaction Metadata was found - */ - private void assertCompactionMetadata(ServerContext ctx, String tableName) { - var tableId = TableId.of(ctx.tableOperations().tableIdMap().get(tableName)); - try (var tabletsMetadata = ctx.getAmple().readTablets().forTable(tableId).build()) { - for (TabletMetadata tablet : tabletsMetadata) { - // TODO: Make this actually look for compaction metadata - assertEquals(tablet, tablet); - } + // Checking the sizes of the files before compaction + for (String tableName : tableNames) { + long sizes = getFileSizes(getCluster().getServerContext(), tableName); + assertTrue(sizes > data.length * 10 && sizes < data.length * 11); } + + // Create two PluginConfigs for the CompressionConfigurer + PluginConfig configurerCompact = new PluginConfig(CompressionConfigurer.class.getName(), + Map.of(CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, data.length + "", + CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); + PluginConfig configurerNoCompact = new PluginConfig(CompressionConfigurer.class.getName(), + Map.of(CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, data.length + 9999 + "", + CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); + + // Compacting the table + client.compactTable(sharedSecret, tableNames[0], null, null, null, true, true, null, + configurerCompact); + client.compactTable(sharedSecret, tableNames[1], null, null, null, true, true, null, + configurerNoCompact); + + // Checking to see that the data sizes are the appropriate size + long sizes1 = getFileSizes(getCluster().getServerContext(), tableNames[0]); + long sizes2 = getFileSizes(getCluster().getServerContext(), tableNames[1]); + assertTrue(sizes1 < data.length); + assertTrue(sizes1 < sizes2, + "Configurer meant to do no compactions is smaller than Configurer that did compactions."); } @Test From 5e1fba5459393c40d7d6022a771c5e3471f35e67 Mon Sep 17 00:00:00 2001 From: Robert Kurc Date: Sun, 13 Jul 2025 21:38:56 +0000 Subject: [PATCH 4/6] Finished test for Compaction Configurer, and confirmed functionality of test. --- .../accumulo/proxy/its/SimpleProxyBase.java | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 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 9437f56..343b783 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -2368,29 +2368,25 @@ public void testCompactionConfigurer() throws Exception { // Checking the sizes of the files before compaction for (String tableName : tableNames) { long sizes = getFileSizes(getCluster().getServerContext(), tableName); - assertTrue(sizes > data.length * 10 && sizes < data.length * 11); + assertTrue(sizes > data.length * 10.0 && sizes < data.length * 11.0); } - // Create two PluginConfigs for the CompressionConfigurer + // Create a PluginConfig for the CompressionConfigurer PluginConfig configurerCompact = new PluginConfig(CompressionConfigurer.class.getName(), Map.of(CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, data.length + "", CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); - PluginConfig configurerNoCompact = new PluginConfig(CompressionConfigurer.class.getName(), - Map.of(CompressionConfigurer.LARGE_FILE_COMPRESSION_THRESHOLD, data.length + 9999 + "", - CompressionConfigurer.LARGE_FILE_COMPRESSION_TYPE, "gz")); - // Compacting the table + // Compacting the tables one with the Configurer, one without client.compactTable(sharedSecret, tableNames[0], null, null, null, true, true, null, configurerCompact); - client.compactTable(sharedSecret, tableNames[1], null, null, null, true, true, null, - configurerNoCompact); + client.compactTable(sharedSecret, tableNames[1], null, null, null, true, true, null, null); - // Checking to see that the data sizes are the appropriate size + // Checking to see that the data sizes are the appropriate size. Based on the data, it will be + // significantly smaller with compression long sizes1 = getFileSizes(getCluster().getServerContext(), tableNames[0]); long sizes2 = getFileSizes(getCluster().getServerContext(), tableNames[1]); assertTrue(sizes1 < data.length); - assertTrue(sizes1 < sizes2, - "Configurer meant to do no compactions is smaller than Configurer that did compactions."); + assertTrue(sizes1 < sizes2, "Size1 is " + sizes1 + ", size2 is " + sizes2); } @Test From 42d9418543174c1e1fe21ec744a5710162682e12 Mon Sep 17 00:00:00 2001 From: Robert Kurc <145068316+Kidcredible300@users.noreply.github.com> Date: Thu, 21 Aug 2025 23:13:29 -0400 Subject: [PATCH 5/6] Check of sizes2 Based on suggestion from Keith Turner, added another check for sizes2. --- src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java | 1 + 1 file changed, 1 insertion(+) 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 343b783..a1de9f0 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -2387,6 +2387,7 @@ public void testCompactionConfigurer() throws Exception { long sizes2 = getFileSizes(getCluster().getServerContext(), tableNames[1]); assertTrue(sizes1 < data.length); assertTrue(sizes1 < sizes2, "Size1 is " + sizes1 + ", size2 is " + sizes2); + assertTrue(sizes2 > data.length * 10.0 && sizes2 < data.length * 11.0); } @Test From 56b8bfdae7b9061c38aa96a0a4dc089d4db1c06d Mon Sep 17 00:00:00 2001 From: Robert Date: Sun, 14 Sep 2025 14:36:55 -0400 Subject: [PATCH 6/6] Wrapped tabletsMetadata in a try-with-resources block. --- .../accumulo/proxy/its/SimpleProxyBase.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 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 a1de9f0..7289586 100644 --- a/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java +++ b/src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java @@ -2324,14 +2324,15 @@ public void testCompactionSelector() throws Exception { */ private long getFileSizes(ServerContext ctx, String tableName) { TableId tableId = TableId.of(ctx.tableOperations().tableIdMap().get(tableName)); - var tabletsMetadata = ctx.getAmple().readTablets().forTable(tableId).build(); - return tabletsMetadata.stream().flatMap(tm -> tm.getFiles().stream()).mapToLong(stf -> { - try { - return FileSystem.getLocal(new Configuration()).getFileStatus(stf.getPath()).getLen(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }).sum(); + try (var tabletsMetadata = ctx.getAmple().readTablets().forTable(tableId).build();) { + return tabletsMetadata.stream().flatMap(tm -> tm.getFiles().stream()).mapToLong(stf -> { + try { + return FileSystem.getLocal(new Configuration()).getFileStatus(stf.getPath()).getLen(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }).sum(); + } } /**