This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push:
new 01b0ff9356 Unit test compaction reservation and deny offline (#4334)
01b0ff9356 is described below
commit 01b0ff9356b851d8f47368989407fa610441c23d
Author: Keith Turner <[email protected]>
AuthorDate: Mon Mar 4 10:30:10 2024 -0500
Unit test compaction reservation and deny offline (#4334)
Changes the compaction coordinator to not return jobs for a table that
is offline. The point where this check was added to the coordinator
needed unit test, so also added the unit test.
---
.../coordinator/CompactionCoordinator.java | 18 ++-
.../compaction/CompactionCoordinatorTest.java | 148 +++++++++++++++++++++
2 files changed, 160 insertions(+), 6 deletions(-)
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java
b/server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java
index ebd5d8f764..a07ff50bc4 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java
@@ -74,6 +74,7 @@ import org.apache.accumulo.core.fate.FateInstanceType;
import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter;
import org.apache.accumulo.core.iterators.user.HasExternalCompactionsFilter;
import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil;
+import org.apache.accumulo.core.manager.state.tables.TableState;
import org.apache.accumulo.core.metadata.CompactableFileImpl;
import org.apache.accumulo.core.metadata.ReferencedTabletFile;
import org.apache.accumulo.core.metadata.StoredTabletFile;
@@ -124,6 +125,7 @@ import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.CacheLoader;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.github.benmanes.caffeine.cache.Weigher;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Sets;
import com.google.common.net.HostAndPort;
@@ -382,9 +384,9 @@ public class CompactionCoordinator
}
- // ELASTICITY_TODO unit test this code
- private boolean canReserveCompaction(TabletMetadata tablet, CompactionJob
job,
- Set<StoredTabletFile> jobFiles) {
+ @VisibleForTesting
+ public static boolean canReserveCompaction(TabletMetadata tablet,
CompactionKind kind,
+ Set<StoredTabletFile> jobFiles, ServerContext ctx) {
if (tablet == null) {
// the tablet no longer exist
@@ -395,6 +397,10 @@ public class CompactionCoordinator
return false;
}
+ if (ctx.getTableState(tablet.getTableId()) != TableState.ONLINE) {
+ return false;
+ }
+
if (!tablet.getFiles().containsAll(jobFiles)) {
return false;
}
@@ -406,7 +412,7 @@ public class CompactionCoordinator
return false;
}
- switch (job.getKind()) {
+ switch (kind) {
case SYSTEM:
var userRequestedCompactions =
tablet.getUserCompactionsRequested().size();
if (userRequestedCompactions > 0) {
@@ -427,7 +433,7 @@ public class CompactionCoordinator
}
break;
default:
- throw new UnsupportedOperationException("Not currently handling " +
job.getKind());
+ throw new UnsupportedOperationException("Not currently handling " +
kind);
}
return true;
@@ -508,7 +514,7 @@ public class CompactionCoordinator
try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
var extent = metaJob.getTabletMetadata().getExtent();
- if (!canReserveCompaction(tabletMetadata, metaJob.getJob(), jobFiles))
{
+ if (!canReserveCompaction(tabletMetadata, metaJob.getJob().getKind(),
jobFiles, ctx)) {
return null;
}
diff --git
a/server/manager/src/test/java/org/apache/accumulo/manager/compaction/CompactionCoordinatorTest.java
b/server/manager/src/test/java/org/apache/accumulo/manager/compaction/CompactionCoordinatorTest.java
index 770405863e..4c0b2b1d52 100644
---
a/server/manager/src/test/java/org/apache/accumulo/manager/compaction/CompactionCoordinatorTest.java
+++
b/server/manager/src/test/java/org/apache/accumulo/manager/compaction/CompactionCoordinatorTest.java
@@ -18,10 +18,18 @@
*/
package org.apache.accumulo.manager.compaction;
+import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.ECOMP;
+import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.OPID;
+import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SELECTED;
+import static
org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.USER_COMPACTION_REQUESTED;
+import static
org.apache.accumulo.manager.compaction.coordinator.CompactionCoordinator.canReserveCompaction;
import static org.easymock.EasyMock.expect;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -47,12 +55,17 @@ import org.apache.accumulo.core.fate.Fate;
import org.apache.accumulo.core.fate.FateId;
import org.apache.accumulo.core.fate.FateInstanceType;
import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil;
+import org.apache.accumulo.core.manager.state.tables.TableState;
import org.apache.accumulo.core.metadata.CompactableFileImpl;
import org.apache.accumulo.core.metadata.ReferencedTabletFile;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.schema.CompactionMetadata;
+import org.apache.accumulo.core.metadata.schema.DataFileValue;
import org.apache.accumulo.core.metadata.schema.ExternalCompactionId;
+import org.apache.accumulo.core.metadata.schema.SelectedFiles;
import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletOperationId;
+import org.apache.accumulo.core.metadata.schema.TabletOperationType;
import org.apache.accumulo.core.securityImpl.thrift.TCredentials;
import org.apache.accumulo.core.spi.compaction.CompactionJob;
import org.apache.accumulo.core.spi.compaction.CompactionKind;
@@ -374,4 +387,139 @@ public class CompactionCoordinatorTest {
EasyMock.verify(context, creds, security);
}
+
+ @Test
+ public void testCanReserve() throws Exception {
+ TableId tableId1 = TableId.of("5");
+ TableId tableId2 = TableId.of("6");
+
+ var file1 = StoredTabletFile.of(new
URI("file:///accumulo/tables/1/default_tablet/F00001.rf"));
+ var file2 = StoredTabletFile.of(new
URI("file:///accumulo/tables/1/default_tablet/F00002.rf"));
+ var file3 = StoredTabletFile.of(new
URI("file:///accumulo/tables/1/default_tablet/F00003.rf"));
+ var file4 = StoredTabletFile.of(new
URI("file:///accumulo/tables/1/default_tablet/F00004.rf"));
+
+ ServerContext context = EasyMock.mock(ServerContext.class);
+
EasyMock.expect(context.getTableState(tableId1)).andReturn(TableState.ONLINE).atLeastOnce();
+
EasyMock.expect(context.getTableState(tableId2)).andReturn(TableState.OFFLINE).atLeastOnce();
+
+ FateId fateId1 = FateId.from(FateInstanceType.USER, 1234L);
+
+ CompactorGroupId cgid = CompactorGroupId.of("G1");
+ ReferencedTabletFile tmp1 =
+ ReferencedTabletFile.of(new
Path("file:///accumulo/tables/1/default_tablet/C00005.rf_tmp"));
+ CompactionMetadata cm1 = new CompactionMetadata(Set.of(file1, file2),
tmp1, "localhost:4444",
+ CompactionKind.SYSTEM, (short) 5, cgid, false, null);
+
+ ReferencedTabletFile tmp2 =
+ ReferencedTabletFile.of(new
Path("file:///accumulo/tables/1/default_tablet/C00006.rf_tmp"));
+ CompactionMetadata cm2 = new CompactionMetadata(Set.of(file3), tmp2,
"localhost:5555",
+ CompactionKind.USER, (short) 5, cgid, false, fateId1);
+
+ EasyMock.replay(context);
+
+ KeyExtent extent1 = new KeyExtent(tableId1, null, null);
+
+ var dfv = new DataFileValue(1000, 100);
+
+ var cid1 = ExternalCompactionId.generate(UUID.randomUUID());
+ var cid2 = ExternalCompactionId.generate(UUID.randomUUID());
+
+ var selected = new SelectedFiles(Set.of(file1, file2, file3), false,
fateId1);
+
+ // should not be able to compact an offline table
+ var tabletOffline = TabletMetadata.builder(new KeyExtent(tableId2, null,
null))
+ .putFile(file1, dfv).putFile(file2, dfv).putFile(file3,
dfv).putFile(file4, dfv)
+ .build(OPID, ECOMP, USER_COMPACTION_REQUESTED, SELECTED);
+ assertFalse(
+ canReserveCompaction(tabletOffline, CompactionKind.SYSTEM,
Set.of(file1, file2), context));
+
+ // nothing should prevent this compaction
+ var tablet1 =
+ TabletMetadata.builder(extent1).putFile(file1, dfv).putFile(file2,
dfv).putFile(file3, dfv)
+ .putFile(file4, dfv).build(OPID, ECOMP, USER_COMPACTION_REQUESTED,
SELECTED);
+ assertTrue(canReserveCompaction(tablet1, CompactionKind.SYSTEM,
Set.of(file1, file2), context));
+
+ // should not be able to do a user compaction unless selected files are
present
+ assertFalse(canReserveCompaction(tablet1, CompactionKind.USER,
Set.of(file1, file2), context));
+
+ // should not be able to compact a tablet with user compaction request in
place
+ var tablet3 =
+ TabletMetadata.builder(extent1).putFile(file1, dfv).putFile(file2,
dfv).putFile(file3, dfv)
+ .putFile(file4,
dfv).putUserCompactionRequested(fateId1).build(OPID, ECOMP, SELECTED);
+ assertFalse(
+ canReserveCompaction(tablet3, CompactionKind.SYSTEM, Set.of(file1,
file2), context));
+
+ // should not be able to compact a tablet when the job has files not
present in the tablet
+ var tablet4 = TabletMetadata.builder(extent1).putFile(file1,
dfv).putFile(file2, dfv)
+ .putFile(file3, dfv).build(OPID, ECOMP, USER_COMPACTION_REQUESTED,
SELECTED);
+ assertFalse(
+ canReserveCompaction(tablet4, CompactionKind.SYSTEM, Set.of(file1,
file2, file4), context));
+
+ // should not be able to compact a tablet with an operation id present
+ TabletOperationId opid =
TabletOperationId.from(TabletOperationType.SPLITTING, fateId1);
+ var tablet5 = TabletMetadata.builder(extent1).putFile(file1,
dfv).putFile(file2, dfv)
+ .putFile(file3, dfv).putFile(file4, dfv).putOperation(opid)
+ .build(ECOMP, USER_COMPACTION_REQUESTED, SELECTED);
+ assertFalse(
+ canReserveCompaction(tablet5, CompactionKind.SYSTEM, Set.of(file1,
file2), context));
+
+ // should not be able to compact a tablet if the job files overlaps with
running compactions
+ var tablet6 = TabletMetadata.builder(extent1).putFile(file1,
dfv).putFile(file2, dfv)
+ .putFile(file3, dfv).putFile(file4, dfv).putExternalCompaction(cid1,
cm1)
+ .putExternalCompaction(cid2, cm2).build(OPID,
USER_COMPACTION_REQUESTED, SELECTED);
+ assertFalse(
+ canReserveCompaction(tablet6, CompactionKind.SYSTEM, Set.of(file1,
file2), context));
+ // should be able to compact the file that is outside of the set of files
currently compacting
+ assertTrue(canReserveCompaction(tablet6, CompactionKind.SYSTEM,
Set.of(file4), context));
+
+ // create a tablet with a selected set of files
+ var selTablet = TabletMetadata.builder(extent1).putFile(file1,
dfv).putFile(file2, dfv)
+ .putFile(file3, dfv).putFile(file4, dfv).putSelectedFiles(selected)
+ .build(OPID, USER_COMPACTION_REQUESTED, ECOMP);
+ // should not be able to start a system compaction if the set of files
overlaps with the
+ // selected files
+ assertFalse(
+ canReserveCompaction(selTablet, CompactionKind.SYSTEM, Set.of(file1,
file2), context));
+ assertFalse(
+ canReserveCompaction(selTablet, CompactionKind.SYSTEM, Set.of(file3,
file4), context));
+ // should be able to start a system compaction on the set of files not in
the selected set
+ assertTrue(canReserveCompaction(selTablet, CompactionKind.SYSTEM,
Set.of(file4), context));
+ // should be able to start user compactions on files that are selected
+ assertTrue(canReserveCompaction(selTablet, CompactionKind.USER,
Set.of(file1, file2), context));
+ assertTrue(canReserveCompaction(selTablet, CompactionKind.USER,
Set.of(file2, file3), context));
+ assertTrue(
+ canReserveCompaction(selTablet, CompactionKind.USER, Set.of(file1,
file2, file3), context));
+ // should not be able to start user compactions on files that fall outside
of the selected set
+ assertFalse(
+ canReserveCompaction(selTablet, CompactionKind.USER, Set.of(file1,
file4), context));
+ assertFalse(canReserveCompaction(selTablet, CompactionKind.USER,
Set.of(file4), context));
+ assertFalse(canReserveCompaction(selTablet, CompactionKind.USER,
+ Set.of(file1, file2, file3, file4), context));
+
+ // test selected files and running compaction
+ var selRunningTablet = TabletMetadata.builder(extent1).putFile(file1,
dfv).putFile(file2, dfv)
+ .putFile(file3, dfv).putFile(file4, dfv).putSelectedFiles(selected)
+ .putExternalCompaction(cid2, cm2).build(OPID,
USER_COMPACTION_REQUESTED);
+ // should be able to compact files that are in the selected set and not in
the running set
+ assertTrue(
+ canReserveCompaction(selRunningTablet, CompactionKind.USER,
Set.of(file1, file2), context));
+ // should not be able to compact because files overlap the running set
+ assertFalse(
+ canReserveCompaction(selRunningTablet, CompactionKind.USER,
Set.of(file2, file3), context));
+ // should not be able to start a system compaction if the set of files
overlaps with the
+ // selected files and/or the running set
+ assertFalse(canReserveCompaction(selRunningTablet, CompactionKind.SYSTEM,
Set.of(file1, file2),
+ context));
+ assertFalse(canReserveCompaction(selRunningTablet, CompactionKind.SYSTEM,
Set.of(file3, file4),
+ context));
+ // should be able to start a system compaction on the set of files not in
the selected set
+ assertTrue(
+ canReserveCompaction(selRunningTablet, CompactionKind.SYSTEM,
Set.of(file4), context));
+
+ // should not be able to compact a tablet that does not exists
+ assertFalse(canReserveCompaction(null, CompactionKind.SYSTEM,
Set.of(file1, file2), context));
+ assertFalse(canReserveCompaction(null, CompactionKind.USER, Set.of(file1,
file2), context));
+
+ EasyMock.verify(context);
+ }
}
