From 4445261f345315f29a6124e67747f611066cbe84 Mon Sep 17 00:00:00 2001 From: Francesco Nigro Date: Mon, 17 Jun 2019 11:52:55 +0200 Subject: [PATCH] ARTEMIS-2382 Reclaimer doesn't need to be instantiatable --- .../core/journal/impl/JournalImpl.java | 8 +- .../artemis/core/journal/impl/Reclaimer.java | 7 +- .../unit/core/journal/impl/ReclaimerTest.java | 93 +++++++++---------- 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java index 203edd3b03..0139367eca 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java @@ -84,6 +84,8 @@ import org.apache.activemq.artemis.utils.collections.ConcurrentLongHashMap; import org.apache.activemq.artemis.utils.collections.ConcurrentLongHashSet; import org.jboss.logging.Logger; +import static org.apache.activemq.artemis.core.journal.impl.Reclaimer.scan; + /** *

A circular log implementation.

*

Look at {@link JournalImpl#load(LoaderCallback)} for the file layout @@ -214,8 +216,6 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal private volatile int compactCount = 0; - private final Reclaimer reclaimer = new Reclaimer(); - public float getCompactPercentage() { return compactPercentage; } @@ -2156,7 +2156,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal break; } try { - reclaimer.scan(getDataFiles()); + scan(getDataFiles()); for (JournalFile file : filesRepository.getDataFiles()) { if (file.isCanReclaim()) { @@ -2249,7 +2249,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal /* Only meant to be used in tests. */ @Override public String debug() throws Exception { - reclaimer.scan(getDataFiles()); + scan(getDataFiles()); StringBuilder builder = new StringBuilder(); diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/Reclaimer.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/Reclaimer.java index 3760723043..5fbe69bee0 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/Reclaimer.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/Reclaimer.java @@ -32,15 +32,18 @@ import org.jboss.logging.Logger; *

2) All pos that correspond to any neg in file Fn, must all live in any file Fm where {@code 0 <= m <= n} * which are also marked for deletion in the same pass of the algorithm.

*/ -public class Reclaimer { +public final class Reclaimer { private static final Logger logger = Logger.getLogger(Reclaimer.class); + private Reclaimer() { + } + // The files are scanned in two stages. First we only check for 2) and do so while that criteria is not met. // When 2) is met, set the first reclaim flag in the journal. After that point only check for 1) // until that criteria is met as well. When 1) is met we set the second flag and the file can be reclaimed. - public void scan(final JournalFile[] files) { + public static void scan(final JournalFile[] files) { for (int i = 0; i < files.length; i++) { JournalFile currentFile = files[i]; diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/ReclaimerTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/ReclaimerTest.java index e4dbad0169..10674ac514 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/ReclaimerTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/ReclaimerTest.java @@ -22,24 +22,21 @@ import java.util.Map; import org.apache.activemq.artemis.core.io.SequentialFile; import org.apache.activemq.artemis.core.journal.impl.JournalFile; import org.apache.activemq.artemis.core.journal.impl.JournalImpl; -import org.apache.activemq.artemis.core.journal.impl.Reclaimer; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.apache.activemq.artemis.core.journal.impl.Reclaimer.scan; + public class ReclaimerTest extends ActiveMQTestBase { private JournalFile[] files; - private Reclaimer reclaimer; - @Override @Before public void setUp() throws Exception { super.setUp(); - - reclaimer = new Reclaimer(); } @Test @@ -48,7 +45,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); } @@ -59,7 +56,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10, 7); - reclaimer.scan(files); + scan(files); assertCantDelete(0); } @@ -70,7 +67,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10); - reclaimer.scan(files); + scan(files); assertCantDelete(0); } @@ -81,7 +78,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 0, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); } @@ -93,7 +90,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10); setupPosNeg(1, 0, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -107,7 +104,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10, 10); setupPosNeg(1, 10, 0, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -121,7 +118,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10, 7); setupPosNeg(1, 10, 3, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -134,7 +131,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10, 10); setupPosNeg(1, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCantDelete(1); @@ -147,7 +144,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10); setupPosNeg(1, 10, 0, 10); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -160,7 +157,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10); setupPosNeg(1, 10); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -173,7 +170,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(0, 10); setupPosNeg(1, 10, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCantDelete(1); @@ -189,7 +186,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 0, 0, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -204,7 +201,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 3, 5, 0); setupPosNeg(2, 10, 0, 5, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -219,7 +216,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 6, 5, 0); setupPosNeg(2, 10, 3, 5, 10); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -234,7 +231,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 6, 5, 0); setupPosNeg(2, 0, 3, 5, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -249,7 +246,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 0, 6, 0, 0); setupPosNeg(2, 0, 3, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -266,7 +263,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 5, 0); setupPosNeg(2, 10, 0, 5, 10); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -281,7 +278,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 5, 0); setupPosNeg(2, 0, 0, 5, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -296,7 +293,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 5, 0); setupPosNeg(2, 0, 0, 5, 10); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -311,7 +308,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 5, 0); setupPosNeg(2, 0, 0, 5, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -328,7 +325,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 0, 0, 2); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -343,7 +340,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 1, 0, 2); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -358,7 +355,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 1, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -373,7 +370,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 0, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -388,7 +385,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 0, 3, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCanDelete(1); @@ -405,7 +402,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 2, 3, 0); setupPosNeg(2, 10, 1, 5, 7); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -420,7 +417,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 0, 2, 0, 0); setupPosNeg(2, 10, 1, 0, 7); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -435,7 +432,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 2, 3, 0); setupPosNeg(2, 0, 1, 5, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -450,7 +447,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 0, 2, 0, 0); setupPosNeg(2, 0, 1, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -465,7 +462,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 3, 0); setupPosNeg(2, 10, 1, 5, 7); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -480,7 +477,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 3, 0); setupPosNeg(2, 10, 1, 0, 7); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -495,7 +492,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 3, 0); setupPosNeg(2, 10, 1, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -510,7 +507,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 0, 0); setupPosNeg(2, 10, 1, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -525,7 +522,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 0, 0); setupPosNeg(2, 10, 0, 0, 0); - reclaimer.scan(files); + scan(files); assertCantDelete(0); assertCantDelete(1); @@ -542,7 +539,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 0, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -557,7 +554,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 10, 0); setupPosNeg(2, 10, 3, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -572,7 +569,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 3, 10, 0); setupPosNeg(2, 10, 3, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -587,7 +584,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 0, 3, 10, 0); setupPosNeg(2, 10, 3, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -602,7 +599,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 0, 3, 10, 0); setupPosNeg(2, 10, 0, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCanDelete(1); @@ -619,7 +616,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 0, 0); setupPosNeg(2, 10, 0, 0, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCantDelete(1); @@ -634,7 +631,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 3, 0); setupPosNeg(2, 10, 0, 0, 5); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCantDelete(1); @@ -649,7 +646,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 3, 0); setupPosNeg(2, 10, 0, 6, 5); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCantDelete(1); @@ -664,7 +661,7 @@ public class ReclaimerTest extends ActiveMQTestBase { setupPosNeg(1, 10, 0, 3, 0); setupPosNeg(2, 0, 0, 6, 0); - reclaimer.scan(files); + scan(files); assertCanDelete(0); assertCantDelete(1);