kevinrr888 commented on code in PR #5881: URL: https://github.com/apache/accumulo/pull/5881#discussion_r2344880796
########## test/src/main/java/org/apache/accumulo/test/SystemTableCompactionIT.java: ########## @@ -0,0 +1,238 @@ +/* + * 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 + * + * https://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.test; + +import static org.apache.accumulo.test.functional.FunctionalTestUtils.getStoredTabletFiles; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.iterators.IteratorUtil; +import org.apache.accumulo.core.metadata.ScanServerRefTabletFile; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.SystemTables; +import org.apache.accumulo.harness.MiniClusterConfigurationCallback; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.functional.SlowIterator; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Sets; +import com.google.common.net.HostAndPort; + +public class SystemTableCompactionIT extends SharedMiniClusterBase { + private static final Logger log = LoggerFactory.getLogger(SystemTableCompactionIT.class); + private static final String SLOW_ITER_NAME = "CustomSlowIter"; + private AccumuloClient client; + private TableOperations ops; + private String userTable; + + public static class IsolatedCompactorsConfig implements MiniClusterConfigurationCallback { + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + cfg.getClusterServerConfiguration().setNumDefaultCompactors(2); + } + } + + @Override + protected Duration defaultTimeout() { + return Duration.ofMinutes(1); + } + + @BeforeAll + public static void setup() throws Exception { + SharedMiniClusterBase.startMiniClusterWithConfig(new IsolatedCompactorsConfig()); + } + + @AfterAll + public static void teardown() { + SharedMiniClusterBase.stopMiniCluster(); + } + + @BeforeEach + public void beforeEach() throws Exception { + client = Accumulo.newClient().from(getClientProps()).build(); + ops = client.tableOperations(); + } + + @AfterEach + public void afterEach() throws Exception { + if (userTable != null) { + cleanupFateTable(); + ops.delete(userTable); + userTable = null; + } + cleanupScanRefTable(); + client.close(); + } + + @Test + public void test_compact() throws Exception { + // disable the GC to prevent removal of newly appeared compilations files due to compaction on + // METADATA and ROOT tables + getCluster().getClusterControl().stopAllServers(ServerType.GARBAGE_COLLECTOR); + try { + userTable = getUniqueNames(1)[0]; + ops.create(userTable); + + // create some RFiles for the METADATA and ROOT tables by creating some data in the user + // table, flushing that table, then the METADATA table, then the ROOT table + for (int i = 0; i < 3; i++) { + try (var bw = client.createBatchWriter(userTable)) { + var mut = new Mutation("r" + i); + mut.put("cf", "cq", "v"); + bw.addMutation(mut); + } + ops.flush(userTable, null, null, true); + ops.flush(SystemTables.METADATA.tableName(), null, null, true); + ops.flush(SystemTables.ROOT.tableName(), null, null, true); + } + + // Create a file for the scan ref and Fate tables + createScanRefTableRow(); + ops.flush(SystemTables.SCAN_REF.tableName(), null, null, true); + createFateTableRow(userTable); + ops.flush(SystemTables.FATE.tableName(), null, null, true); + + for (var sysTable : SystemTables.tableNames()) { + Set<StoredTabletFile> stfsBeforeCompact = + getStoredTabletFiles(getCluster().getServerContext(), client, sysTable); + + log.info("Compacting {} with files: {}", sysTable, stfsBeforeCompact); + // This compaction will run in the 'system_compactors' group. Review Comment: `// This compaction will run in the 'system_compactors' group.` is this true in current impl? ########## test/src/main/java/org/apache/accumulo/test/ComprehensiveTableOperationsIT_SimpleSuite.java: ########## @@ -371,11 +371,6 @@ public void test_merge() throws Exception { assertTrue(ops.listSplits(SystemTables.SCAN_REF.tableName()).isEmpty()); } - @Test - public void test_compact() throws Exception { - // TODO see issue#5679 - } - Review Comment: ComprehensiveTableOperationsIT_SimpleSuite will fail with this change as it asserts all table operations have an associated test. I think it might be better to include the test here instead of a new IT, remove "_SimpleSuite" from the class name, and include `cfg.getClusterServerConfiguration().setNumDefaultCompactors(2);` for the configuration of these tests. Can maybe make note that the number of default compactors is set to 2 specifically for test_compact Regardless of these test changes, I think this IT should be removed from SimpleSuite anyways since it does weird things to system tables, which may affect other tests in the SimpleSuite ########## test/src/main/java/org/apache/accumulo/test/SystemTableCompactionIT.java: ########## @@ -0,0 +1,238 @@ +/* + * 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 + * + * https://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.test; + +import static org.apache.accumulo.test.functional.FunctionalTestUtils.getStoredTabletFiles; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.iterators.IteratorUtil; +import org.apache.accumulo.core.metadata.ScanServerRefTabletFile; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.SystemTables; +import org.apache.accumulo.harness.MiniClusterConfigurationCallback; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.functional.SlowIterator; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Sets; +import com.google.common.net.HostAndPort; + +public class SystemTableCompactionIT extends SharedMiniClusterBase { + private static final Logger log = LoggerFactory.getLogger(SystemTableCompactionIT.class); + private static final String SLOW_ITER_NAME = "CustomSlowIter"; + private AccumuloClient client; + private TableOperations ops; + private String userTable; + + public static class IsolatedCompactorsConfig implements MiniClusterConfigurationCallback { + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + cfg.getClusterServerConfiguration().setNumDefaultCompactors(2); + } + } + + @Override + protected Duration defaultTimeout() { + return Duration.ofMinutes(1); + } + + @BeforeAll + public static void setup() throws Exception { + SharedMiniClusterBase.startMiniClusterWithConfig(new IsolatedCompactorsConfig()); + } + + @AfterAll + public static void teardown() { + SharedMiniClusterBase.stopMiniCluster(); + } + + @BeforeEach + public void beforeEach() throws Exception { + client = Accumulo.newClient().from(getClientProps()).build(); + ops = client.tableOperations(); + } + + @AfterEach + public void afterEach() throws Exception { + if (userTable != null) { + cleanupFateTable(); + ops.delete(userTable); + userTable = null; + } + cleanupScanRefTable(); + client.close(); + } + + @Test + public void test_compact() throws Exception { + // disable the GC to prevent removal of newly appeared compilations files due to compaction on + // METADATA and ROOT tables + getCluster().getClusterControl().stopAllServers(ServerType.GARBAGE_COLLECTOR); + try { + userTable = getUniqueNames(1)[0]; + ops.create(userTable); + + // create some RFiles for the METADATA and ROOT tables by creating some data in the user + // table, flushing that table, then the METADATA table, then the ROOT table + for (int i = 0; i < 3; i++) { + try (var bw = client.createBatchWriter(userTable)) { + var mut = new Mutation("r" + i); + mut.put("cf", "cq", "v"); + bw.addMutation(mut); + } + ops.flush(userTable, null, null, true); + ops.flush(SystemTables.METADATA.tableName(), null, null, true); + ops.flush(SystemTables.ROOT.tableName(), null, null, true); + } + + // Create a file for the scan ref and Fate tables + createScanRefTableRow(); + ops.flush(SystemTables.SCAN_REF.tableName(), null, null, true); + createFateTableRow(userTable); + ops.flush(SystemTables.FATE.tableName(), null, null, true); + + for (var sysTable : SystemTables.tableNames()) { + Set<StoredTabletFile> stfsBeforeCompact = + getStoredTabletFiles(getCluster().getServerContext(), client, sysTable); + + log.info("Compacting {} with files: {}", sysTable, stfsBeforeCompact); + // This compaction will run in the 'system_compactors' group. + // Set wait to false to avoid blocking on the FATE transaction, which can be slow. + ops.compact(sysTable, null, null, true, false); + log.info("Initiated compaction for " + sysTable); Review Comment: ```suggestion log.info("Completed compaction for " + sysTable); ``` ########## test/src/main/java/org/apache/accumulo/test/ComprehensiveTableOperationsIT_SimpleSuite.java: ########## @@ -371,11 +371,6 @@ public void test_merge() throws Exception { assertTrue(ops.listSplits(SystemTables.SCAN_REF.tableName()).isEmpty()); } - @Test - public void test_compact() throws Exception { - // TODO see issue#5679 - } - Review Comment: When adding this back here, would be good to include ``` // compact for user tables is tested in various ITs. One example is CompactionIT. Ensure // test exists assertDoesNotThrow(() -> Class.forName(CompactionIT.class.getName())); ``` again ########## test/src/main/java/org/apache/accumulo/test/SystemTableCompactionIT.java: ########## @@ -0,0 +1,238 @@ +/* + * 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 + * + * https://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.test; + +import static org.apache.accumulo.test.functional.FunctionalTestUtils.getStoredTabletFiles; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.iterators.IteratorUtil; +import org.apache.accumulo.core.metadata.ScanServerRefTabletFile; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.SystemTables; +import org.apache.accumulo.harness.MiniClusterConfigurationCallback; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.functional.SlowIterator; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Sets; +import com.google.common.net.HostAndPort; + +public class SystemTableCompactionIT extends SharedMiniClusterBase { + private static final Logger log = LoggerFactory.getLogger(SystemTableCompactionIT.class); + private static final String SLOW_ITER_NAME = "CustomSlowIter"; + private AccumuloClient client; + private TableOperations ops; + private String userTable; + + public static class IsolatedCompactorsConfig implements MiniClusterConfigurationCallback { + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + cfg.getClusterServerConfiguration().setNumDefaultCompactors(2); + } + } + + @Override + protected Duration defaultTimeout() { + return Duration.ofMinutes(1); + } + + @BeforeAll + public static void setup() throws Exception { + SharedMiniClusterBase.startMiniClusterWithConfig(new IsolatedCompactorsConfig()); + } + + @AfterAll + public static void teardown() { + SharedMiniClusterBase.stopMiniCluster(); + } + + @BeforeEach + public void beforeEach() throws Exception { + client = Accumulo.newClient().from(getClientProps()).build(); + ops = client.tableOperations(); + } + + @AfterEach + public void afterEach() throws Exception { + if (userTable != null) { + cleanupFateTable(); + ops.delete(userTable); + userTable = null; + } + cleanupScanRefTable(); + client.close(); + } + + @Test + public void test_compact() throws Exception { + // disable the GC to prevent removal of newly appeared compilations files due to compaction on + // METADATA and ROOT tables Review Comment: ```suggestion // disable the GC to prevent automatic compactions on METADATA and ROOT tables ``` ########## test/src/main/java/org/apache/accumulo/test/SystemTableCompactionIT.java: ########## @@ -0,0 +1,238 @@ +/* + * 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 + * + * https://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.test; + +import static org.apache.accumulo.test.functional.FunctionalTestUtils.getStoredTabletFiles; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.iterators.IteratorUtil; +import org.apache.accumulo.core.metadata.ScanServerRefTabletFile; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.SystemTables; +import org.apache.accumulo.harness.MiniClusterConfigurationCallback; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.functional.SlowIterator; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Sets; +import com.google.common.net.HostAndPort; + +public class SystemTableCompactionIT extends SharedMiniClusterBase { + private static final Logger log = LoggerFactory.getLogger(SystemTableCompactionIT.class); + private static final String SLOW_ITER_NAME = "CustomSlowIter"; + private AccumuloClient client; + private TableOperations ops; + private String userTable; + + public static class IsolatedCompactorsConfig implements MiniClusterConfigurationCallback { + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + cfg.getClusterServerConfiguration().setNumDefaultCompactors(2); + } + } + + @Override + protected Duration defaultTimeout() { + return Duration.ofMinutes(1); + } + + @BeforeAll + public static void setup() throws Exception { + SharedMiniClusterBase.startMiniClusterWithConfig(new IsolatedCompactorsConfig()); + } + + @AfterAll + public static void teardown() { + SharedMiniClusterBase.stopMiniCluster(); + } + + @BeforeEach + public void beforeEach() throws Exception { + client = Accumulo.newClient().from(getClientProps()).build(); + ops = client.tableOperations(); + } + + @AfterEach + public void afterEach() throws Exception { + if (userTable != null) { + cleanupFateTable(); + ops.delete(userTable); + userTable = null; + } + cleanupScanRefTable(); + client.close(); + } + + @Test + public void test_compact() throws Exception { + // disable the GC to prevent removal of newly appeared compilations files due to compaction on + // METADATA and ROOT tables + getCluster().getClusterControl().stopAllServers(ServerType.GARBAGE_COLLECTOR); + try { + userTable = getUniqueNames(1)[0]; + ops.create(userTable); + + // create some RFiles for the METADATA and ROOT tables by creating some data in the user + // table, flushing that table, then the METADATA table, then the ROOT table + for (int i = 0; i < 3; i++) { + try (var bw = client.createBatchWriter(userTable)) { + var mut = new Mutation("r" + i); + mut.put("cf", "cq", "v"); + bw.addMutation(mut); + } + ops.flush(userTable, null, null, true); + ops.flush(SystemTables.METADATA.tableName(), null, null, true); + ops.flush(SystemTables.ROOT.tableName(), null, null, true); + } + + // Create a file for the scan ref and Fate tables + createScanRefTableRow(); + ops.flush(SystemTables.SCAN_REF.tableName(), null, null, true); + createFateTableRow(userTable); + ops.flush(SystemTables.FATE.tableName(), null, null, true); + + for (var sysTable : SystemTables.tableNames()) { + Set<StoredTabletFile> stfsBeforeCompact = + getStoredTabletFiles(getCluster().getServerContext(), client, sysTable); + + log.info("Compacting {} with files: {}", sysTable, stfsBeforeCompact); + // This compaction will run in the 'system_compactors' group. + // Set wait to false to avoid blocking on the FATE transaction, which can be slow. + ops.compact(sysTable, null, null, true, false); + log.info("Initiated compaction for " + sysTable); + + // RFiles resulting from a compaction begin with 'A'. Wait until we see an RFile beginning + // with 'A' that was not present before the compaction. + Wait.waitFor(() -> { + var stfsAfterCompact = + getStoredTabletFiles(getCluster().getServerContext(), client, sysTable); + log.info("Completed compaction for {} with new files {}", sysTable, stfsAfterCompact); + String regex = "^A.*\\.rf$"; + var aStfsBeforeCompaction = stfsBeforeCompact.stream() + .filter(stf -> stf.getFileName().matches(regex)).collect(Collectors.toSet()); + var aStfsAfterCompaction = stfsAfterCompact.stream() + .filter(stf -> stf.getFileName().matches(regex)).collect(Collectors.toSet()); + return !Sets.difference(aStfsAfterCompaction, aStfsBeforeCompaction).isEmpty(); + }, TimeUnit.SECONDS.toMillis(90), TimeUnit.SECONDS.toMillis(2)); Review Comment: ```suggestion }); ``` maybe this extra timing config isn't necessary anymore. ########## test/src/main/java/org/apache/accumulo/test/functional/FunctionalTestUtils.java: ########## @@ -89,6 +89,22 @@ public static int countRFiles(AccumuloClient c, String tableName) throws Excepti } } + public static List<String> getRFilePaths(ServerContext context, AccumuloClient client, + String tableName) { + return getStoredTabletFiles(context, client, tableName).stream() + .map(StoredTabletFile::getMetadataPath).collect(Collectors.toList()); + } + + public static Set<StoredTabletFile> getStoredTabletFiles(ServerContext context, + AccumuloClient client, String tableName) { + TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName)); + try (var tabletsMetadata = context.getAmple().readTablets().forTable(tableId).build()) { + return tabletsMetadata.stream().flatMap(tm -> tm.getFiles().stream()) + .collect(Collectors.toSet()); + } + } + + Review Comment: See Christophers comments on #5878 ########## test/src/main/java/org/apache/accumulo/test/SystemTableCompactionIT.java: ########## @@ -0,0 +1,238 @@ +/* + * 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 + * + * https://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.test; + +import static org.apache.accumulo.test.functional.FunctionalTestUtils.getStoredTabletFiles; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.iterators.IteratorUtil; +import org.apache.accumulo.core.metadata.ScanServerRefTabletFile; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.SystemTables; +import org.apache.accumulo.harness.MiniClusterConfigurationCallback; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.test.functional.ReadWriteIT; +import org.apache.accumulo.test.functional.SlowIterator; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Sets; +import com.google.common.net.HostAndPort; + +public class SystemTableCompactionIT extends SharedMiniClusterBase { + private static final Logger log = LoggerFactory.getLogger(SystemTableCompactionIT.class); + private static final String SLOW_ITER_NAME = "CustomSlowIter"; + private AccumuloClient client; + private TableOperations ops; + private String userTable; + + public static class IsolatedCompactorsConfig implements MiniClusterConfigurationCallback { + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + cfg.getClusterServerConfiguration().setNumDefaultCompactors(2); + } + } + + @Override + protected Duration defaultTimeout() { + return Duration.ofMinutes(1); + } + + @BeforeAll + public static void setup() throws Exception { + SharedMiniClusterBase.startMiniClusterWithConfig(new IsolatedCompactorsConfig()); + } + + @AfterAll + public static void teardown() { + SharedMiniClusterBase.stopMiniCluster(); + } + + @BeforeEach + public void beforeEach() throws Exception { + client = Accumulo.newClient().from(getClientProps()).build(); + ops = client.tableOperations(); + } + + @AfterEach + public void afterEach() throws Exception { + if (userTable != null) { + cleanupFateTable(); + ops.delete(userTable); + userTable = null; + } + cleanupScanRefTable(); + client.close(); + } + + @Test + public void test_compact() throws Exception { + // disable the GC to prevent removal of newly appeared compilations files due to compaction on + // METADATA and ROOT tables + getCluster().getClusterControl().stopAllServers(ServerType.GARBAGE_COLLECTOR); + try { + userTable = getUniqueNames(1)[0]; + ops.create(userTable); + + // create some RFiles for the METADATA and ROOT tables by creating some data in the user + // table, flushing that table, then the METADATA table, then the ROOT table + for (int i = 0; i < 3; i++) { + try (var bw = client.createBatchWriter(userTable)) { + var mut = new Mutation("r" + i); + mut.put("cf", "cq", "v"); + bw.addMutation(mut); + } + ops.flush(userTable, null, null, true); + ops.flush(SystemTables.METADATA.tableName(), null, null, true); + ops.flush(SystemTables.ROOT.tableName(), null, null, true); + } + + // Create a file for the scan ref and Fate tables + createScanRefTableRow(); + ops.flush(SystemTables.SCAN_REF.tableName(), null, null, true); + createFateTableRow(userTable); + ops.flush(SystemTables.FATE.tableName(), null, null, true); + + for (var sysTable : SystemTables.tableNames()) { + Set<StoredTabletFile> stfsBeforeCompact = + getStoredTabletFiles(getCluster().getServerContext(), client, sysTable); + + log.info("Compacting {} with files: {}", sysTable, stfsBeforeCompact); + // This compaction will run in the 'system_compactors' group. + // Set wait to false to avoid blocking on the FATE transaction, which can be slow. + ops.compact(sysTable, null, null, true, false); Review Comment: ```suggestion ops.compact(sysTable, null, null, true, true); ``` I think we should be waiting here. The compactions on the system tables should be able to complete (now that there are 2 compactors), it's the compaction on the user table that will be slow. -- 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]
