This is an automated email from the ASF dual-hosted git repository. dlmarion pushed a commit to branch 1451-external-compactions-feature in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/1451-external-compactions-feature by this push: new 237decd Fixed some spotbugs errors 237decd is described below commit 237decd3cb7d5317cf3896ce220fd5742c4eca2f Author: Dave Marion <dlmar...@apache.org> AuthorDate: Tue Apr 27 16:06:54 2021 +0000 Fixed some spotbugs errors --- server/compaction-coordinator/pom.xml | 17 ++++ .../accumulo/coordinator/QueueSummariesTest.java | 104 +++++++++++---------- .../apache/accumulo/test/ExternalCompactionIT.java | 60 ++++++------ 3 files changed, 101 insertions(+), 80 deletions(-) diff --git a/server/compaction-coordinator/pom.xml b/server/compaction-coordinator/pom.xml index 4ed49e3..0793491 100644 --- a/server/compaction-coordinator/pom.xml +++ b/server/compaction-coordinator/pom.xml @@ -36,6 +36,14 @@ <optional>true</optional> </dependency> <dependency> + <groupId>com.google.code.gson</groupId> + <artifactId>gson</artifactId> + </dependency> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + </dependency> + <dependency> <groupId>org.apache.accumulo</groupId> <artifactId>accumulo-core</artifactId> </dependency> @@ -64,6 +72,15 @@ <artifactId>jetty-server</artifactId> </dependency> <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-util</artifactId> + </dependency> + <dependency> + <groupId>org.eclipse.jetty.toolchain</groupId> + <artifactId>jetty-jakarta-servlet-api</artifactId> + <version>5.0.2</version> + </dependency> + <dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-api</artifactId> </dependency> diff --git a/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/QueueSummariesTest.java b/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/QueueSummariesTest.java index 7b69680..bafb9b5 100644 --- a/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/QueueSummariesTest.java +++ b/server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/QueueSummariesTest.java @@ -18,6 +18,9 @@ */ package org.apache.accumulo.coordinator; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -25,7 +28,6 @@ import java.util.Set; import org.apache.accumulo.coordinator.QueueSummaries.PrioTserver; import org.apache.accumulo.core.metadata.TServerInstance; import org.apache.accumulo.core.tabletserver.thrift.TCompactionQueueSummary; -import org.junit.Assert; import org.junit.Test; public class QueueSummariesTest { @@ -60,33 +62,33 @@ public class QueueSummariesTest { update(queueSum, "ts3", "q1", "5", "q2", "5"); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts3", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts3", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts3", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts3", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); } queueSum.removeSummary(ntsi("ts2"), "q1", 5); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts3", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts3", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts3", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts3", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); } queueSum.removeSummary(ntsi("ts3"), "q2", 5); queueSum.removeSummary(ntsi("ts2"), "q3", 5); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts3", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 4), queueSum.getNextTserver("q3")); - Assert.assertEquals(npt("ts2", 4), queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts3", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 4), queueSum.getNextTserver("q3")); + assertEquals(npt("ts2", 4), queueSum.getNextTserver("q3")); } } @@ -99,10 +101,10 @@ public class QueueSummariesTest { update(queueSum, "ts2", "q1", "5", "q2", "4", "q3", "5"); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); } // an update from the tserver should remove some existing entries @@ -110,20 +112,20 @@ public class QueueSummariesTest { update(queueSum, "ts2", "q1", "7", "q3", "3", "q4", "5"); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts2", 7), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 6), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 3), queueSum.getNextTserver("q3")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q4")); + assertEquals(npt("ts2", 7), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 6), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 3), queueSum.getNextTserver("q3")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q4")); } queueSum.removeSummary(ntsi("ts2"), "q1", 7); queueSum.removeSummary(ntsi("ts1"), "q2", 6); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 4), queueSum.getNextTserver("q1")); - Assert.assertNull(queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 3), queueSum.getNextTserver("q3")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q4")); + assertEquals(npt("ts1", 4), queueSum.getNextTserver("q1")); + assertNull(queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 3), queueSum.getNextTserver("q3")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q4")); } } @@ -135,58 +137,58 @@ public class QueueSummariesTest { update(queueSum, "ts2", "q1", "5", "q2", "4", "q3", "5"); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts1", 5), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); } queueSum.remove(Set.of(ntsi("ts1"))); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts2", 4), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); + assertEquals(npt("ts2", 4), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q3")); } queueSum.removeSummary(ntsi("ts2"), "q3", 5); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts2", 4), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); - Assert.assertNull(queueSum.getNextTserver("q3")); + assertEquals(npt("ts2", 4), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); + assertNull(queueSum.getNextTserver("q3")); } update(queueSum, "ts1", "q2", "6", "q1", "3"); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 6), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); - Assert.assertNull(queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 6), queueSum.getNextTserver("q2")); + assertEquals(npt("ts2", 5), queueSum.getNextTserver("q1")); + assertNull(queueSum.getNextTserver("q3")); } queueSum.remove(Set.of(ntsi("ts2"))); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 6), queueSum.getNextTserver("q2")); - Assert.assertEquals(npt("ts1", 3), queueSum.getNextTserver("q1")); - Assert.assertNull(queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 6), queueSum.getNextTserver("q2")); + assertEquals(npt("ts1", 3), queueSum.getNextTserver("q1")); + assertNull(queueSum.getNextTserver("q3")); } queueSum.removeSummary(ntsi("ts1"), "q2", 6); for (int i = 0; i < 3; i++) { - Assert.assertEquals(npt("ts1", 3), queueSum.getNextTserver("q1")); - Assert.assertNull(queueSum.getNextTserver("q2")); - Assert.assertNull(queueSum.getNextTserver("q3")); + assertEquals(npt("ts1", 3), queueSum.getNextTserver("q1")); + assertNull(queueSum.getNextTserver("q2")); + assertNull(queueSum.getNextTserver("q3")); } queueSum.remove(Set.of(ntsi("ts1"))); for (int i = 0; i < 3; i++) { - Assert.assertNull(queueSum.getNextTserver("q1")); - Assert.assertNull(queueSum.getNextTserver("q2")); - Assert.assertNull(queueSum.getNextTserver("q3")); + assertNull(queueSum.getNextTserver("q1")); + assertNull(queueSum.getNextTserver("q2")); + assertNull(queueSum.getNextTserver("q3")); } } } diff --git a/test/src/main/java/org/apache/accumulo/test/ExternalCompactionIT.java b/test/src/main/java/org/apache/accumulo/test/ExternalCompactionIT.java index 4e7faa8..726bcd4 100644 --- a/test/src/main/java/org/apache/accumulo/test/ExternalCompactionIT.java +++ b/test/src/main/java/org/apache/accumulo/test/ExternalCompactionIT.java @@ -19,6 +19,9 @@ package org.apache.accumulo.test; import static org.apache.accumulo.minicluster.ServerType.TABLET_SERVER; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.net.URI; @@ -79,7 +82,6 @@ import org.apache.accumulo.test.functional.ConfigurableMacBase; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.RawLocalFileSystem; import org.apache.hadoop.io.Text; -import org.junit.Assert; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -226,10 +228,10 @@ public class ExternalCompactionIT extends ConfigurableMacBase { // Check that there is one failed compaction in the coordinator metrics ExternalCompactionMetrics metrics = getCoordinatorMetrics(); - Assert.assertEquals(1, metrics.getStarted()); - Assert.assertEquals(1, metrics.getRunning()); // CBUG: Should be zero when #2032 is resolved - Assert.assertEquals(0, metrics.getCompleted()); - Assert.assertEquals(1, metrics.getFailed()); + assertEquals(1, metrics.getStarted()); + assertEquals(1, metrics.getRunning()); // CBUG: Should be zero when #2032 is resolved + assertEquals(0, metrics.getCompleted()); + assertEquals(1, metrics.getFailed()); } @@ -276,7 +278,7 @@ public class ExternalCompactionIT extends ConfigurableMacBase { tm = getCluster().getServerContext().getAmple().readTablets().forTable(tid) .fetch(ColumnType.PREV_ROW).build(); tm.forEach(t -> md.add(t)); - Assert.assertEquals(2, md.size()); + assertEquals(2, md.size()); Text start = md.get(0).getPrevEndRow(); Text end = md.get(1).getEndRow(); @@ -288,8 +290,8 @@ public class ExternalCompactionIT extends ConfigurableMacBase { tm = getCluster().getServerContext().getAmple().readTablets().forTable(tid) .fetch(ColumnType.ECOMP).build(); tm.forEach(t -> md.add(t)); - Assert.assertEquals(0, md.size()); - Assert.assertEquals(0, + assertEquals(0, md.size()); + assertEquals(0, getCluster().getServerContext().getAmple().getExternalCompactionFinalStates().count()); // Wait for the table to merge by waiting for only 1 tablet to show up in the metadata table @@ -300,10 +302,10 @@ public class ExternalCompactionIT extends ConfigurableMacBase { // Check that there is one failed compaction in the coordinator metrics ExternalCompactionMetrics metrics = getCoordinatorMetrics(); - Assert.assertTrue(metrics.getStarted() > 0); - Assert.assertTrue(metrics.getRunning() > 0); // CBUG: Should be zero when #2032 is resolved - Assert.assertEquals(0, metrics.getCompleted()); - Assert.assertTrue(metrics.getFailed() > 0); + assertTrue(metrics.getStarted() > 0); + assertTrue(metrics.getRunning() > 0); // CBUG: Should be zero when #2032 is resolved + assertEquals(0, metrics.getCompleted()); + assertTrue(metrics.getFailed() > 0); } @@ -374,10 +376,10 @@ public class ExternalCompactionIT extends ConfigurableMacBase { // The metadata tablets will be deleted from the metadata table because we have deleted the // table. Verify that the compaction failed by looking at the metrics in the Coordinator. ExternalCompactionMetrics metrics = getCoordinatorMetrics(); - Assert.assertEquals(1, metrics.getStarted()); - Assert.assertEquals(1, metrics.getRunning()); // CBUG: Should be zero when #2032 is resolved - Assert.assertEquals(0, metrics.getCompleted()); - Assert.assertEquals(1, metrics.getFailed()); + assertEquals(1, metrics.getStarted()); + assertEquals(1, metrics.getRunning()); // CBUG: Should be zero when #2032 is resolved + assertEquals(0, metrics.getCompleted()); + assertEquals(1, metrics.getFailed()); } } @@ -423,10 +425,10 @@ public class ExternalCompactionIT extends ConfigurableMacBase { // The metadata tablets will be deleted from the metadata table because we have deleted the // table. Verify that the compaction failed by looking at the metrics in the Coordinator. ExternalCompactionMetrics metrics = getCoordinatorMetrics(); - Assert.assertEquals(1, metrics.getStarted()); - Assert.assertEquals(1, metrics.getRunning()); // CBUG: Should be zero when #2032 is resolved - Assert.assertEquals(0, metrics.getCompleted()); - Assert.assertEquals(1, metrics.getFailed()); + assertEquals(1, metrics.getStarted()); + assertEquals(1, metrics.getRunning()); // CBUG: Should be zero when #2032 is resolved + assertEquals(0, metrics.getCompleted()); + assertEquals(1, metrics.getFailed()); } } @@ -442,7 +444,7 @@ public class ExternalCompactionIT extends ConfigurableMacBase { try { getCluster().killProcess(TABLET_SERVER, p); } catch (Exception e) { - Assert.fail("Failed to shutdown tablet server"); + fail("Failed to shutdown tablet server"); } }); // Start our TServer that will not commit the compaction @@ -472,15 +474,15 @@ public class ExternalCompactionIT extends ConfigurableMacBase { .forLevel(DataLevel.USER).fetch(ColumnType.ECOMP).build(); List<TabletMetadata> md = new ArrayList<>(); tm.forEach(t -> md.add(t)); - Assert.assertEquals(1, md.size()); + assertEquals(1, md.size()); TabletMetadata m = md.get(0); Map<ExternalCompactionId,ExternalCompactionMetadata> em = m.getExternalCompactions(); - Assert.assertEquals(1, em.size()); + assertEquals(1, em.size()); List<ExternalCompactionFinalState> finished = new ArrayList<>(); getCluster().getServerContext().getAmple().getExternalCompactionFinalStates() .forEach(f -> finished.add(f)); - Assert.assertEquals(1, finished.size()); - Assert.assertEquals(em.entrySet().iterator().next().getKey(), + assertEquals(1, finished.size()); + assertEquals(em.entrySet().iterator().next().getKey(), finished.get(0).getExternalCompactionId()); tm.close(); @@ -523,9 +525,9 @@ public class ExternalCompactionIT extends ConfigurableMacBase { HttpClient hc = HttpClient.newBuilder().version(Version.HTTP_1_1).followRedirects(Redirect.NORMAL).build(); HttpResponse<String> res = hc.send(req, BodyHandlers.ofString()); - Assert.assertEquals(200, res.statusCode()); + assertEquals(200, res.statusCode()); String metrics = res.body(); - Assert.assertNotNull(metrics); + assertNotNull(metrics); System.out.println("Metrics response: " + metrics); return new Gson().fromJson(metrics, ExternalCompactionMetrics.class); } @@ -535,7 +537,7 @@ public class ExternalCompactionIT extends ConfigurableMacBase { try (Scanner scanner = client.createScanner(table1)) { int count = 0; for (Entry<Key,Value> entry : scanner) { - Assert.assertTrue(Integer.parseInt(entry.getValue().toString()) % modulus == 0); + assertTrue(Integer.parseInt(entry.getValue().toString()) % modulus == 0); count++; } @@ -545,7 +547,7 @@ public class ExternalCompactionIT extends ConfigurableMacBase { expectedCount++; } - Assert.assertEquals(expectedCount, count); + assertEquals(expectedCount, count); } }