From f404f6834cb280bac256a89a4852a41f4e9be24e Mon Sep 17 00:00:00 2001 From: Kevin Rathbun Date: Thu, 24 Oct 2024 13:16:01 -0400 Subject: [PATCH] Refactors MultipleStoresIT (#4975) * Refactors MultipleStoresIT Refactors MultipleStoresIT to function more similarly to how other fate tests are run (the two store types are tested in their own separate concrete classes). MultipleStoresIT is now an abstract class implemented by two new classes MetaMultipleStoresIT and UserMultipleStoresIT. closes #4903 --- .../accumulo/test/fate/MultipleStoresIT.java | 186 +++++------------- .../test/fate/meta/MetaMultipleStoresIT.java | 82 ++++++++ .../test/fate/user/UserMultipleStoresIT.java | 95 +++++++++ 3 files changed, 228 insertions(+), 135 deletions(-) create mode 100644 test/src/main/java/org/apache/accumulo/test/fate/meta/MetaMultipleStoresIT.java create mode 100644 test/src/main/java/org/apache/accumulo/test/fate/user/UserMultipleStoresIT.java diff --git a/test/src/main/java/org/apache/accumulo/test/fate/MultipleStoresIT.java b/test/src/main/java/org/apache/accumulo/test/fate/MultipleStoresIT.java index edd6a538597..292f1fdfb43 100644 --- a/test/src/main/java/org/apache/accumulo/test/fate/MultipleStoresIT.java +++ b/test/src/main/java/org/apache/accumulo/test/fate/MultipleStoresIT.java @@ -18,13 +18,11 @@ */ package org.apache.accumulo.test.fate; -import static org.apache.accumulo.test.fate.user.UserFateStoreIT.createFateTable; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.File; import java.time.Duration; import java.util.ArrayList; import java.util.Collections; @@ -38,94 +36,44 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; -import org.apache.accumulo.core.client.Accumulo; -import org.apache.accumulo.core.clientImpl.ClientContext; import org.apache.accumulo.core.conf.DefaultConfiguration; import org.apache.accumulo.core.conf.Property; 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.fate.FateStore; import org.apache.accumulo.core.fate.ReadOnlyFateStore; import org.apache.accumulo.core.fate.Repo; -import org.apache.accumulo.core.fate.user.UserFateStore; -import org.apache.accumulo.core.fate.zookeeper.MetaFateStore; -import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.core.fate.zookeeper.ZooUtil; import org.apache.accumulo.harness.SharedMiniClusterBase; import org.apache.accumulo.test.util.Wait; -import org.apache.accumulo.test.zookeeper.ZooKeeperTestingServer; -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.apache.zookeeper.KeeperException; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.collect.Sets; -public class MultipleStoresIT extends SharedMiniClusterBase { +public abstract class MultipleStoresIT extends SharedMiniClusterBase { private static final Logger LOG = LoggerFactory.getLogger(MultipleStoresIT.class); - @TempDir - private static File tempDir; - private static ZooKeeperTestingServer szk = null; - private static ZooReaderWriter zk; - private static final String FATE_DIR = "/fate"; - private ClientContext client; - - @BeforeEach - public void beforeEachSetup() { - client = (ClientContext) Accumulo.newClient().from(getClientProps()).build(); - } - - @AfterEach - public void afterEachTeardown() { - client.close(); - } - - @BeforeAll - public static void beforeAllSetup() throws Exception { - SharedMiniClusterBase.startMiniCluster(); - szk = new ZooKeeperTestingServer(tempDir); - zk = szk.getZooReaderWriter(); - } - - @AfterAll - public static void afterAllTeardown() throws Exception { - SharedMiniClusterBase.stopMiniCluster(); - szk.close(); - } @Test public void testReserveUnreserve() throws Exception { - testReserveUnreserve(FateInstanceType.META); - testReserveUnreserve(FateInstanceType.USER); + executeSleepingEnvTest(this::testReserveUnreserve); } - private void testReserveUnreserve(FateInstanceType storeType) throws Exception { + private void testReserveUnreserve(TestStoreFactory testStoreFactory) + throws Exception { // reserving/unreserving a FateId should be reflected across instances of the stores - final String tableName = getUniqueNames(1)[0]; final int numFateIds = 500; - final FateId fakeFateId = FateId.from(storeType, UUID.randomUUID()); final List> reservations = new ArrayList<>(); - final boolean isUserStore = storeType == FateInstanceType.USER; final Set allIds = new HashSet<>(); - final FateStore store1, store2; final ZooUtil.LockID lock1 = new ZooUtil.LockID("/locks", "L1", 50); final ZooUtil.LockID lock2 = new ZooUtil.LockID("/locks", "L2", 52); Map activeReservations; - - if (isUserStore) { - createFateTable(client, tableName); - store1 = new UserFateStore<>(client, tableName, lock1, null); - store2 = new UserFateStore<>(client, tableName, lock2, null); - } else { - store1 = new MetaFateStore<>(FATE_DIR, zk, lock1, null); - store2 = new MetaFateStore<>(FATE_DIR, zk, lock2, null); - } + final FateStore store1 = testStoreFactory.create(lock1, null); + final FateStore store2 = testStoreFactory.create(lock2, null); + final FateId fakeFateId = FateId.from(store1.type(), UUID.randomUUID()); // Create the fate ids using store1 for (int i = 0; i < numFateIds; i++) { @@ -174,25 +122,16 @@ private void testReserveUnreserve(FateInstanceType storeType) throws Exception { @Test public void testReserveNonExistentTxn() throws Exception { - testReserveNonExistentTxn(FateInstanceType.META); - testReserveNonExistentTxn(FateInstanceType.USER); + executeSleepingEnvTest(this::testReserveNonExistentTxn); } - private void testReserveNonExistentTxn(FateInstanceType storeType) throws Exception { + private void testReserveNonExistentTxn(TestStoreFactory testStoreFactory) + throws Exception { // Tests that reserve() doesn't hang indefinitely and instead throws an error // on reserve() a non-existent transaction. - final FateStore store; - final boolean isUserStore = storeType == FateInstanceType.USER; - final String tableName = getUniqueNames(1)[0]; - final FateId fakeFateId = FateId.from(storeType, UUID.randomUUID()); final ZooUtil.LockID lock = new ZooUtil.LockID("/locks", "L1", 50); - - if (isUserStore) { - createFateTable(client, tableName); - store = new UserFateStore<>(client, tableName, lock, null); - } else { - store = new MetaFateStore<>(FATE_DIR, zk, lock, null); - } + final FateStore store = testStoreFactory.create(lock, null); + final FateId fakeFateId = FateId.from(store.type(), UUID.randomUUID()); var err = assertThrows(IllegalStateException.class, () -> store.reserve(fakeFateId)); assertTrue(err.getMessage().contains(fakeFateId.canonical())); @@ -200,26 +139,16 @@ private void testReserveNonExistentTxn(FateInstanceType storeType) throws Except @Test public void testReserveReservedAndUnreserveUnreserved() throws Exception { - testReserveReservedAndUnreserveUnreserved(FateInstanceType.META); - testReserveReservedAndUnreserveUnreserved(FateInstanceType.USER); + executeSleepingEnvTest(this::testReserveReservedAndUnreserveUnreserved); } - private void testReserveReservedAndUnreserveUnreserved(FateInstanceType storeType) - throws Exception { - final String tableName = getUniqueNames(1)[0]; + private void testReserveReservedAndUnreserveUnreserved( + TestStoreFactory testStoreFactory) throws Exception { final int numFateIds = 500; - final boolean isUserStore = storeType == FateInstanceType.USER; final Set allIds = new HashSet<>(); final List> reservations = new ArrayList<>(); - final FateStore store; final ZooUtil.LockID lock = new ZooUtil.LockID("/locks", "L1", 50); - - if (isUserStore) { - createFateTable(client, tableName); - store = new UserFateStore<>(client, tableName, lock, null); - } else { - store = new MetaFateStore<>(FATE_DIR, zk, lock, null); - } + final FateStore store = testStoreFactory.create(lock, null); // Create some FateIds and ensure that they can be reserved for (int i = 0; i < numFateIds; i++) { @@ -248,26 +177,16 @@ private void testReserveReservedAndUnreserveUnreserved(FateInstanceType storeTyp @Test public void testReserveAfterUnreserveAndReserveAfterDeleted() throws Exception { - testReserveAfterUnreserveAndReserveAfterDeleted(FateInstanceType.META); - testReserveAfterUnreserveAndReserveAfterDeleted(FateInstanceType.USER); + executeSleepingEnvTest(this::testReserveAfterUnreserveAndReserveAfterDeleted); } - private void testReserveAfterUnreserveAndReserveAfterDeleted(FateInstanceType storeType) - throws Exception { - final String tableName = getUniqueNames(1)[0]; + private void testReserveAfterUnreserveAndReserveAfterDeleted( + TestStoreFactory testStoreFactory) throws Exception { final int numFateIds = 500; - final boolean isUserStore = storeType == FateInstanceType.USER; final Set allIds = new HashSet<>(); final List> reservations = new ArrayList<>(); - final FateStore store; final ZooUtil.LockID lock = new ZooUtil.LockID("/locks", "L1", 50); - - if (isUserStore) { - createFateTable(client, tableName); - store = new UserFateStore<>(client, tableName, lock, null); - } else { - store = new MetaFateStore<>(FATE_DIR, zk, lock, null); - } + final FateStore store = testStoreFactory.create(lock, null); // Create some FateIds and ensure that they can be reserved for (int i = 0; i < numFateIds; i++) { @@ -305,31 +224,22 @@ private void testReserveAfterUnreserveAndReserveAfterDeleted(FateInstanceType st @Test public void testMultipleFateInstances() throws Exception { - testMultipleFateInstances(FateInstanceType.META); - testMultipleFateInstances(FateInstanceType.USER); + executeSleepingEnvTest(this::testMultipleFateInstances); } - private void testMultipleFateInstances(FateInstanceType storeType) throws Exception { - final String tableName = getUniqueNames(1)[0]; + private void testMultipleFateInstances(TestStoreFactory testStoreFactory) + throws Exception { final int numFateIds = 500; - final boolean isUserStore = storeType == FateInstanceType.USER; final Set allIds = new HashSet<>(); - final FateStore store1, store2; final SleepingTestEnv testEnv1 = new SleepingTestEnv(50); final SleepingTestEnv testEnv2 = new SleepingTestEnv(50); final ZooUtil.LockID lock1 = new ZooUtil.LockID("/locks", "L1", 50); final ZooUtil.LockID lock2 = new ZooUtil.LockID("/locks", "L2", 52); final Set liveLocks = new HashSet<>(); final Predicate isLockHeld = liveLocks::contains; + final FateStore store1 = testStoreFactory.create(lock1, isLockHeld); + final FateStore store2 = testStoreFactory.create(lock2, isLockHeld); - if (isUserStore) { - createFateTable(client, tableName); - store1 = new UserFateStore<>(client, tableName, lock1, isLockHeld); - store2 = new UserFateStore<>(client, tableName, lock2, isLockHeld); - } else { - store1 = new MetaFateStore<>(FATE_DIR, zk, lock1, isLockHeld); - store2 = new MetaFateStore<>(FATE_DIR, zk, lock2, isLockHeld); - } liveLocks.add(lock1); liveLocks.add(lock2); @@ -366,23 +276,20 @@ private void testMultipleFateInstances(FateInstanceType storeType) throws Except @Test public void testDeadReservationsCleanup() throws Exception { - testDeadReservationsCleanup(FateInstanceType.META); - testDeadReservationsCleanup(FateInstanceType.USER); + executeLatchEnvTest(this::testDeadReservationsCleanup); } - private void testDeadReservationsCleanup(FateInstanceType storeType) throws Exception { + private void testDeadReservationsCleanup(TestStoreFactory testStoreFactory) + throws Exception { // Tests reserving some transactions, then simulating that the Manager died by creating // a new Fate instance and store with a new LockID. The transactions which were // reserved using the old LockID should be cleaned up by Fate's DeadReservationCleaner, // then picked up by the new Fate/store. - final String tableName = getUniqueNames(1)[0]; // One transaction for each FATE worker thread final int numFateIds = Integer.parseInt(Property.MANAGER_FATE_THREADPOOL_SIZE.getDefaultValue()); - final boolean isUserStore = storeType == FateInstanceType.USER; final Set allIds = new HashSet<>(); - final FateStore store1, store2; final LatchTestEnv testEnv1 = new LatchTestEnv(); final LatchTestEnv testEnv2 = new LatchTestEnv(); final ZooUtil.LockID lock1 = new ZooUtil.LockID("/locks", "L1", 50); @@ -391,12 +298,7 @@ private void testDeadReservationsCleanup(FateInstanceType storeType) throws Exce final Predicate isLockHeld = liveLocks::contains; Map reservations; - if (isUserStore) { - createFateTable(client, tableName); - store1 = new UserFateStore<>(client, tableName, lock1, isLockHeld); - } else { - store1 = new MetaFateStore<>(FATE_DIR, zk, lock1, isLockHeld); - } + final FateStore store1 = testStoreFactory.create(lock1, isLockHeld); liveLocks.add(lock1); FastFate fate1 = new FastFate<>(testEnv1, store1, true, Object::toString, @@ -423,11 +325,7 @@ private void testDeadReservationsCleanup(FateInstanceType storeType) throws Exce assertEquals(allIds, reservations.keySet()); reservations.values().forEach(res -> assertEquals(lock1, res.getLockID())); - if (isUserStore) { - store2 = new UserFateStore<>(client, tableName, lock2, isLockHeld); - } else { - store2 = new MetaFateStore<>(FATE_DIR, zk, lock2, isLockHeld); - } + final FateStore store2 = testStoreFactory.create(lock2, isLockHeld); // Verify store2 can see the reserved transactions even though they were reserved using // store1 @@ -499,7 +397,7 @@ public String getReturn() { } } - public static class SleepingTestEnv { + public static class SleepingTestEnv extends MultipleStoresTestEnv { public final Set executedOps = Collections.synchronizedSet(new HashSet<>()); public final int sleepTimeMs; @@ -542,8 +440,26 @@ public String getReturn() { } } - public static class LatchTestEnv { + public static class LatchTestEnv extends MultipleStoresTestEnv { public final AtomicInteger numWorkers = new AtomicInteger(0); public final CountDownLatch workersLatch = new CountDownLatch(1); } + + protected abstract void executeSleepingEnvTest( + MultipleStoresTestExecutor testMethod) throws Exception; + + protected abstract void executeLatchEnvTest(MultipleStoresTestExecutor testMethod) + throws Exception; + + protected interface TestStoreFactory { + FateStore create(ZooUtil.LockID lockID, Predicate isLockHeld) + throws InterruptedException, KeeperException; + } + + @FunctionalInterface + protected interface MultipleStoresTestExecutor { + void execute(TestStoreFactory fateStoreFactory) throws Exception; + } + + protected static class MultipleStoresTestEnv {} } diff --git a/test/src/main/java/org/apache/accumulo/test/fate/meta/MetaMultipleStoresIT.java b/test/src/main/java/org/apache/accumulo/test/fate/meta/MetaMultipleStoresIT.java new file mode 100644 index 00000000000..a2866cb900f --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/fate/meta/MetaMultipleStoresIT.java @@ -0,0 +1,82 @@ +/* + * 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.fate.meta; + +import java.io.File; +import java.util.function.Predicate; + +import org.apache.accumulo.core.Constants; +import org.apache.accumulo.core.fate.FateStore; +import org.apache.accumulo.core.fate.zookeeper.MetaFateStore; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.fate.zookeeper.ZooUtil; +import org.apache.accumulo.test.fate.MultipleStoresIT; +import org.apache.accumulo.test.zookeeper.ZooKeeperTestingServer; +import org.apache.zookeeper.KeeperException; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.io.TempDir; + +public class MetaMultipleStoresIT extends MultipleStoresIT { + @TempDir + private static File TEMP_DIR; + private static ZooKeeperTestingServer SZK; + private static ZooReaderWriter ZK; + private static String FATE_DIR; + + @BeforeAll + public static void setup() throws Exception { + SZK = new ZooKeeperTestingServer(TEMP_DIR); + ZK = SZK.getZooReaderWriter(); + FATE_DIR = Constants.ZFATE; + } + + @AfterAll + public static void teardown() throws Exception { + SZK.close(); + } + + @Override + protected void executeSleepingEnvTest(MultipleStoresTestExecutor testMethod) + throws Exception { + testMethod.execute(new SleepingEnvMetaStoreFactory()); + } + + @Override + protected void executeLatchEnvTest(MultipleStoresTestExecutor testMethod) + throws Exception { + testMethod.execute(new LatchEnvMetaStoreFactory()); + } + + static class SleepingEnvMetaStoreFactory implements TestStoreFactory { + @Override + public FateStore create(ZooUtil.LockID lockID, + Predicate isLockHeld) throws InterruptedException, KeeperException { + return new MetaFateStore<>(FATE_DIR, ZK, lockID, isLockHeld); + } + } + + static class LatchEnvMetaStoreFactory implements TestStoreFactory { + @Override + public FateStore create(ZooUtil.LockID lockID, + Predicate isLockHeld) throws InterruptedException, KeeperException { + return new MetaFateStore<>(FATE_DIR, ZK, lockID, isLockHeld); + } + } +} diff --git a/test/src/main/java/org/apache/accumulo/test/fate/user/UserMultipleStoresIT.java b/test/src/main/java/org/apache/accumulo/test/fate/user/UserMultipleStoresIT.java new file mode 100644 index 00000000000..f3569f07aab --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/fate/user/UserMultipleStoresIT.java @@ -0,0 +1,95 @@ +/* + * 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.fate.user; + +import static org.apache.accumulo.test.fate.user.UserFateStoreIT.createFateTable; + +import java.util.function.Predicate; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloException; +import org.apache.accumulo.core.client.AccumuloSecurityException; +import org.apache.accumulo.core.client.TableNotFoundException; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.fate.FateStore; +import org.apache.accumulo.core.fate.user.UserFateStore; +import org.apache.accumulo.core.fate.zookeeper.ZooUtil; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.test.fate.MultipleStoresIT; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; + +public class UserMultipleStoresIT extends MultipleStoresIT { + private ClientContext client; + private String tableName; + + @BeforeAll + public static void beforeAllSetup() throws Exception { + SharedMiniClusterBase.startMiniCluster(); + } + + @BeforeEach + public void beforeEachSetup() throws Exception { + tableName = getUniqueNames(1)[0]; + client = (ClientContext) Accumulo.newClient().from(getClientProps()).build(); + createFateTable(client, tableName); + } + + @AfterAll + public static void afterAllTeardown() { + SharedMiniClusterBase.stopMiniCluster(); + } + + @AfterEach + public void afterEachTeardown() + throws AccumuloException, TableNotFoundException, AccumuloSecurityException { + client.tableOperations().delete(tableName); + client.close(); + } + + @Override + protected void executeSleepingEnvTest(MultipleStoresTestExecutor testMethod) + throws Exception { + testMethod.execute(new SleepingEnvUserStoreFactory()); + } + + @Override + protected void executeLatchEnvTest(MultipleStoresTestExecutor testMethod) + throws Exception { + testMethod.execute(new LatchEnvUserStoreFactory()); + } + + class SleepingEnvUserStoreFactory implements TestStoreFactory { + @Override + public FateStore create(ZooUtil.LockID lockID, + Predicate isLockHeld) { + return new UserFateStore<>(client, tableName, lockID, isLockHeld); + } + } + + class LatchEnvUserStoreFactory implements TestStoreFactory { + @Override + public FateStore create(ZooUtil.LockID lockID, + Predicate isLockHeld) { + return new UserFateStore<>(client, tableName, lockID, isLockHeld); + } + } +}