From 7b893011b2afde302cc341276e0b14ae03feea6f Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Mon, 8 Jan 2024 20:58:51 -0500 Subject: [PATCH 1/2] uses table id in compaction prioritization Makes the default compaction planner use the table id when prioritizing compactions. The root table id is always given the highest priority, followed by the metadata table, then user tables. Fixes #4033 --- .../compaction/DefaultCompactionPlanner.java | 4 +- .../compaction/CompactionJobPrioritizer.java | 56 ++++++--- .../DefaultCompactionPlannerTest.java | 3 + .../compaction/CompactionPrioritizerTest.java | 116 +++++++++++++++--- 4 files changed, 138 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java b/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java index 4cb1f3a4785..2396ad85d32 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java @@ -432,8 +432,8 @@ private List> findFilesToCompactWithLowerRatio( private static short createPriority(PlanningParameters params, Collection group) { - return CompactionJobPrioritizer.createPriority(params.getKind(), params.getAll().size(), - group.size()); + return CompactionJobPrioritizer.createPriority(params.getTableId(), params.getKind(), + params.getAll().size(), group.size()); } private long getMaxSizeToCompact(CompactionKind kind) { diff --git a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java index 261349cb6f8..8147ef3ee23 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java +++ b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java @@ -20,40 +20,56 @@ import java.util.Comparator; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.metadata.schema.Ample; import org.apache.accumulo.core.spi.compaction.CompactionJob; import org.apache.accumulo.core.spi.compaction.CompactionKind; +import com.google.common.base.Preconditions; + public class CompactionJobPrioritizer { public static final Comparator JOB_COMPARATOR = Comparator.comparingInt(CompactionJob::getPriority) .thenComparingInt(job -> job.getFiles().size()).reversed(); - public static short createPriority(CompactionKind kind, int totalFiles, int compactingFiles) { + public static short createPriority(TableId tableId, CompactionKind kind, int totalFiles, + int compactingFiles) { + + Preconditions.checkArgument(totalFiles >= 0, "totalFiles is negative %s", totalFiles); + Preconditions.checkArgument(compactingFiles >= 0, "compactingFiles is negative %s", + compactingFiles); - int prio = totalFiles + compactingFiles; + // This holds the two bits used to encode the priority of the table. + int tablePrefix; - switch (kind) { + switch (Ample.DataLevel.of(tableId)) { + case ROOT: + tablePrefix = 0b0100_0000_0000_0000; + break; + case METADATA: + tablePrefix = 0; + break; case USER: - // user-initiated compactions will have a positive priority - // based on number of files - if (prio > Short.MAX_VALUE) { - return Short.MAX_VALUE; - } - return (short) prio; - case SELECTOR: - case SYSTEM: - // system-initiated compactions will have a negative priority - // starting at -32768 and increasing based on number of files - // maxing out at -1 - if (prio > Short.MAX_VALUE) { - return -1; - } else { - return (short) (Short.MIN_VALUE + prio); - } + tablePrefix = 0b1000_0000_0000_0000; + break; default: - throw new AssertionError("Unknown kind " + kind); + throw new IllegalStateException("Unknown data level" + Ample.DataLevel.of(tableId)); } + + int kindBit; + + if (kind == CompactionKind.USER) { + kindBit = 0b0010_0000_0000_0000; + } else { + kindBit = 0; + } + + int fileBits = Math.min(0b0001_1111_1111_1111, totalFiles + compactingFiles); + + // Encode the table, kind, and files into a short using two bits for the table, one bit for the + // kind, and 13 bits for the file count. + return (short) (tablePrefix | kindBit | fileBits); } } diff --git a/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java b/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java index 265778f1af9..d0f3e77eacd 100644 --- a/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java @@ -51,6 +51,7 @@ import org.apache.accumulo.core.spi.compaction.CompactionPlanner.InitParameters; import org.apache.accumulo.core.util.ConfigurationImpl; import org.apache.accumulo.core.util.compaction.CompactionJobImpl; +import org.apache.accumulo.core.util.compaction.CompactionJobPrioritizer; import org.apache.accumulo.core.util.compaction.CompactionPlanImpl; import org.apache.accumulo.core.util.compaction.CompactionPlannerInitParams; import org.apache.accumulo.core.util.compaction.CompactorGroupIdImpl; @@ -201,6 +202,8 @@ public void testUserCompaction() { var job = getOnlyElement(plan.getJobs()); assertEquals(candidates, job.getFiles()); assertEquals(CompactorGroupIdImpl.groupId("medium"), job.getGroup()); + assertEquals(CompactionJobPrioritizer.createPriority(TableId.of("42"), CompactionKind.USER, + all.size(), job.getFiles().size()), job.getPriority()); // should only run one user compaction at a time compacting = Set.of(createJob(CompactionKind.USER, all, createCFs("F1", "3M", "F2", "3M"))); diff --git a/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java b/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java index a112dbd6b15..e37873eecf3 100644 --- a/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java @@ -18,7 +18,10 @@ */ package org.apache.accumulo.core.util.compaction; +import static org.apache.accumulo.core.util.compaction.CompactionJobPrioritizer.createPriority; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.net.URI; import java.util.ArrayList; @@ -28,6 +31,9 @@ import java.util.Optional; import org.apache.accumulo.core.client.admin.compaction.CompactableFile; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.metadata.MetadataTable; +import org.apache.accumulo.core.metadata.RootTable; import org.apache.accumulo.core.spi.compaction.CompactionJob; import org.apache.accumulo.core.spi.compaction.CompactionKind; import org.junit.jupiter.api.Test; @@ -42,29 +48,101 @@ public CompactionJob createJob(CompactionKind kind, String tablet, int numFiles, .create(URI.create("hdfs://foonn/accumulo/tables/5/" + tablet + "/" + i + ".rf"), 4, 4)); } // TODO pass numFiles - return new CompactionJobImpl( - CompactionJobPrioritizer.createPriority(kind, totalFiles, numFiles), + return new CompactionJobImpl(createPriority(TableId.of("1"), kind, totalFiles, numFiles), CompactorGroupIdImpl.groupId("test"), files, kind, Optional.of(false)); } @Test - public void testPrioritizer() throws Exception { - assertEquals((short) 0, CompactionJobPrioritizer.createPriority(CompactionKind.USER, 0, 0)); - assertEquals((short) 10000, - CompactionJobPrioritizer.createPriority(CompactionKind.USER, 10000, 0)); - assertEquals((short) 32767, - CompactionJobPrioritizer.createPriority(CompactionKind.USER, 32767, 0)); - assertEquals((short) 32767, - CompactionJobPrioritizer.createPriority(CompactionKind.USER, Integer.MAX_VALUE, 0)); - - assertEquals((short) -32768, - CompactionJobPrioritizer.createPriority(CompactionKind.SYSTEM, 0, 0)); - assertEquals((short) -22768, - CompactionJobPrioritizer.createPriority(CompactionKind.SYSTEM, 10000, 0)); - assertEquals((short) -1, - CompactionJobPrioritizer.createPriority(CompactionKind.SYSTEM, 32767, 0)); - assertEquals((short) -1, - CompactionJobPrioritizer.createPriority(CompactionKind.SYSTEM, Integer.MAX_VALUE, 0)); + public void testOrdering() { + short pr1 = createPriority(RootTable.ID, CompactionKind.USER, 10000, 1); + assertEquals(Short.MAX_VALUE, pr1); + short pr2 = createPriority(RootTable.ID, CompactionKind.USER, 1000, 30); + assertTrue(pr1 > pr2); + short pr3 = createPriority(RootTable.ID, CompactionKind.USER, 1000, 1); + assertTrue(pr2 > pr3); + short pr4 = createPriority(RootTable.ID, CompactionKind.USER, 1, 1); + assertTrue(pr3 > pr4); + short pr5 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 10000, 1); + assertTrue(pr4 > pr5); + short pr6 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1000, 30); + assertTrue(pr5 > pr6); + short pr7 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1000, 1); + assertTrue(pr6 > pr7); + short pr8 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1, 1); + assertTrue(pr7 > pr8); + + short pm1 = createPriority(MetadataTable.ID, CompactionKind.USER, 10000, 1); + assertTrue(pr8 > pm1); + short pm2 = createPriority(MetadataTable.ID, CompactionKind.USER, 1000, 30); + assertTrue(pm1 > pm2); + short pm3 = createPriority(MetadataTable.ID, CompactionKind.USER, 1000, 1); + assertTrue(pm2 > pm3); + short pm4 = createPriority(MetadataTable.ID, CompactionKind.USER, 1, 1); + assertTrue(pm3 > pm4); + short pm5 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 10000, 1); + assertTrue(pm4 > pm5); + short pm6 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1000, 30); + assertTrue(pm5 > pm6); + short pm7 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1000, 1); + assertTrue(pm6 > pm7); + short pm8 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1, 1); + assertTrue(pm7 > pm8); + + var userTable1 = TableId.of("1"); + var userTable2 = TableId.of("2"); + + short pu1 = createPriority(userTable1, CompactionKind.USER, 10000, 1); + assertTrue(pm8 > pu1); + short pu2 = createPriority(userTable2, CompactionKind.USER, 1000, 30); + assertTrue(pu1 > pu2); + short pu3 = createPriority(userTable1, CompactionKind.USER, 1000, 1); + assertTrue(pu2 > pu3); + short pu4 = createPriority(userTable2, CompactionKind.USER, 1, 1); + assertTrue(pu3 > pu4); + short pu5 = createPriority(userTable1, CompactionKind.SYSTEM, 10000, 1); + assertTrue(pu4 > pu5); + short pu6 = createPriority(userTable2, CompactionKind.SYSTEM, 1000, 30); + assertTrue(pu5 > pu6); + short pu7 = createPriority(userTable1, CompactionKind.SYSTEM, 1000, 1); + assertTrue(pu6 > pu7); + short pu8 = createPriority(userTable2, CompactionKind.SYSTEM, 1, 1); + assertTrue(pu7 > pu8); + assertEquals(Short.MIN_VALUE + 2, pu8); + } + + @Test + public void testBoundary() { + // test the boundary condition around the max number of files to encode + int maxFiles = (1 << 13) - 1; + for (var tableId : List.of(TableId.of("1"), TableId.of("2"), RootTable.ID, MetadataTable.ID)) { + for (var kind : CompactionKind.values()) { + short p1 = createPriority(tableId, kind, maxFiles + 10, 10); + short p2 = createPriority(tableId, kind, maxFiles + 10, 5); + assertEquals(p1, p2); + short p3 = createPriority(tableId, kind, maxFiles - 2, 5); + assertEquals(p1, p3); + short p4 = createPriority(tableId, kind, maxFiles - 5, 5); + assertEquals(p1, p4); + short p5 = createPriority(tableId, kind, maxFiles - 6, 5); + assertEquals(p1 - 1, p5); + short p6 = createPriority(tableId, kind, maxFiles - 7, 5); + assertEquals(p1 - 2, p6); + short p7 = createPriority(tableId, kind, maxFiles - 17, 15); + assertEquals(p1 - 2, p7); + short p8 = createPriority(tableId, kind, 1, 1); + assertEquals(p1 - maxFiles + 2, p8); + } + } + } + + @Test + public void testNegative() { + for (var tableId : List.of(TableId.of("1"), TableId.of("2"), RootTable.ID, MetadataTable.ID)) { + for (var kind : CompactionKind.values()) { + assertThrows(IllegalArgumentException.class, () -> createPriority(tableId, kind, -5, 2)); + assertThrows(IllegalArgumentException.class, () -> createPriority(tableId, kind, 10, -5)); + } + } } @Test From 1a2c6dbb16ea3becb396de9bee508df4a58a76bf Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Tue, 9 Jan 2024 21:01:18 +0000 Subject: [PATCH 2/2] use ranges instead of bit fields --- .../compaction/CompactionJobPrioritizer.java | 53 ++++++++++++----- .../compaction/CompactionPrioritizerTest.java | 58 ++++++++++--------- 2 files changed, 67 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java index 8147ef3ee23..0232a6bcc75 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java +++ b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java @@ -33,6 +33,19 @@ public class CompactionJobPrioritizer { Comparator.comparingInt(CompactionJob::getPriority) .thenComparingInt(job -> job.getFiles().size()).reversed(); + private static final short ROOT_USER_MAX = Short.MAX_VALUE; + private static final short ROOT_USER_MIN = ROOT_USER_MAX - 1000; + private static final short ROOT_SYSTEM_MAX = ROOT_USER_MIN - 1; + private static final short ROOT_SYSTEM_MIN = ROOT_SYSTEM_MAX - 1000; + private static final short METADATA_USER_MAX = ROOT_SYSTEM_MIN - 1; + private static final short METADATA_USER_MIN = METADATA_USER_MAX - 1000; + private static final short METADATA_SYSTEM_MAX = METADATA_USER_MIN - 1; + private static final short METADATA_SYSTEM_MIN = METADATA_SYSTEM_MAX - 1000; + private static final short USER_USER_MAX = METADATA_SYSTEM_MIN - 1; + private static final short USER_USER_MIN = USER_USER_MAX - 30768; + private static final short USER_SYSTEM_MAX = USER_USER_MIN - 1; + private static final short USER_SYSTEM_MIN = Short.MIN_VALUE; + public static short createPriority(TableId tableId, CompactionKind kind, int totalFiles, int compactingFiles) { @@ -40,36 +53,44 @@ public static short createPriority(TableId tableId, CompactionKind kind, int tot Preconditions.checkArgument(compactingFiles >= 0, "compactingFiles is negative %s", compactingFiles); + int min; + int max; // This holds the two bits used to encode the priority of the table. int tablePrefix; switch (Ample.DataLevel.of(tableId)) { case ROOT: - tablePrefix = 0b0100_0000_0000_0000; + if (kind == CompactionKind.USER) { + min = ROOT_USER_MIN; + max = ROOT_USER_MAX; + } else { + min = ROOT_SYSTEM_MIN; + max = ROOT_SYSTEM_MAX; + } break; case METADATA: - tablePrefix = 0; + if (kind == CompactionKind.USER) { + min = METADATA_USER_MIN; + max = METADATA_USER_MAX; + } else { + min = METADATA_SYSTEM_MIN; + max = METADATA_SYSTEM_MAX; + } break; case USER: - tablePrefix = 0b1000_0000_0000_0000; + if (kind == CompactionKind.USER) { + min = USER_USER_MIN; + max = USER_USER_MAX; + } else { + min = USER_SYSTEM_MIN; + max = USER_SYSTEM_MAX; + } break; default: throw new IllegalStateException("Unknown data level" + Ample.DataLevel.of(tableId)); } - int kindBit; - - if (kind == CompactionKind.USER) { - kindBit = 0b0010_0000_0000_0000; - } else { - kindBit = 0; - } - - int fileBits = Math.min(0b0001_1111_1111_1111, totalFiles + compactingFiles); - - // Encode the table, kind, and files into a short using two bits for the table, one bit for the - // kind, and 13 bits for the file count. - return (short) (tablePrefix | kindBit | fileBits); + return (short) Math.min(max, min + totalFiles + compactingFiles); } } diff --git a/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java b/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java index e37873eecf3..f727b8512b0 100644 --- a/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java @@ -56,34 +56,34 @@ public CompactionJob createJob(CompactionKind kind, String tablet, int numFiles, public void testOrdering() { short pr1 = createPriority(RootTable.ID, CompactionKind.USER, 10000, 1); assertEquals(Short.MAX_VALUE, pr1); - short pr2 = createPriority(RootTable.ID, CompactionKind.USER, 1000, 30); + short pr2 = createPriority(RootTable.ID, CompactionKind.USER, 100, 30); assertTrue(pr1 > pr2); - short pr3 = createPriority(RootTable.ID, CompactionKind.USER, 1000, 1); + short pr3 = createPriority(RootTable.ID, CompactionKind.USER, 100, 1); assertTrue(pr2 > pr3); short pr4 = createPriority(RootTable.ID, CompactionKind.USER, 1, 1); assertTrue(pr3 > pr4); short pr5 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 10000, 1); assertTrue(pr4 > pr5); - short pr6 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1000, 30); + short pr6 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 100, 30); assertTrue(pr5 > pr6); - short pr7 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1000, 1); + short pr7 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 100, 1); assertTrue(pr6 > pr7); short pr8 = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1, 1); assertTrue(pr7 > pr8); short pm1 = createPriority(MetadataTable.ID, CompactionKind.USER, 10000, 1); assertTrue(pr8 > pm1); - short pm2 = createPriority(MetadataTable.ID, CompactionKind.USER, 1000, 30); + short pm2 = createPriority(MetadataTable.ID, CompactionKind.USER, 100, 30); assertTrue(pm1 > pm2); - short pm3 = createPriority(MetadataTable.ID, CompactionKind.USER, 1000, 1); + short pm3 = createPriority(MetadataTable.ID, CompactionKind.USER, 100, 1); assertTrue(pm2 > pm3); short pm4 = createPriority(MetadataTable.ID, CompactionKind.USER, 1, 1); assertTrue(pm3 > pm4); short pm5 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 10000, 1); assertTrue(pm4 > pm5); - short pm6 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1000, 30); + short pm6 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 100, 30); assertTrue(pm5 > pm6); - short pm7 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1000, 1); + short pm7 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 100, 1); assertTrue(pm6 > pm7); short pm8 = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1, 1); assertTrue(pm7 > pm8); @@ -112,27 +112,29 @@ public void testOrdering() { @Test public void testBoundary() { - // test the boundary condition around the max number of files to encode - int maxFiles = (1 << 13) - 1; - for (var tableId : List.of(TableId.of("1"), TableId.of("2"), RootTable.ID, MetadataTable.ID)) { - for (var kind : CompactionKind.values()) { - short p1 = createPriority(tableId, kind, maxFiles + 10, 10); - short p2 = createPriority(tableId, kind, maxFiles + 10, 5); - assertEquals(p1, p2); - short p3 = createPriority(tableId, kind, maxFiles - 2, 5); - assertEquals(p1, p3); - short p4 = createPriority(tableId, kind, maxFiles - 5, 5); - assertEquals(p1, p4); - short p5 = createPriority(tableId, kind, maxFiles - 6, 5); - assertEquals(p1 - 1, p5); - short p6 = createPriority(tableId, kind, maxFiles - 7, 5); - assertEquals(p1 - 2, p6); - short p7 = createPriority(tableId, kind, maxFiles - 17, 15); - assertEquals(p1 - 2, p7); - short p8 = createPriority(tableId, kind, 1, 1); - assertEquals(p1 - maxFiles + 2, p8); - } + var userTable = TableId.of("1"); + + short minRootUser = createPriority(RootTable.ID, CompactionKind.USER, 1, 1); + short minRootSystem = createPriority(RootTable.ID, CompactionKind.SYSTEM, 1, 1); + short minMetaUser = createPriority(MetadataTable.ID, CompactionKind.USER, 1, 1); + short minMetaSystem = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, 1, 1); + short minUserUser = createPriority(userTable, CompactionKind.USER, 1, 1); + + // Test the boundary condition around the max number of files to encode. Ensure the next level + // is always greater no matter how many files. + for (int files = 1; files < 100_000; files += 1) { + short rootSystem = createPriority(RootTable.ID, CompactionKind.SYSTEM, files, 1); + assertTrue(minRootUser > rootSystem); + short metaUser = createPriority(MetadataTable.ID, CompactionKind.USER, files, 1); + assertTrue(minRootSystem > metaUser); + short metaSystem = createPriority(MetadataTable.ID, CompactionKind.SYSTEM, files, 1); + assertTrue(minMetaUser > metaSystem); + short userUser = createPriority(userTable, CompactionKind.USER, files, 1); + assertTrue(minMetaSystem > userUser); + short userSystem = createPriority(userTable, CompactionKind.SYSTEM, files, 1); + assertTrue(minUserUser > userSystem); } + } @Test