You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/01/12 07:30:11 UTC

[GitHub] sijie closed pull request #967: ISSUE #956: gc: add more stats

sijie closed pull request #967: ISSUE #956: gc: add more stats
URL: https://github.com/apache/bookkeeper/pull/967
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java
index 2e104cd56..7e1969c77 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java
@@ -101,12 +101,22 @@
     String JOURNAL_QUEUE_SIZE = "JOURNAL_QUEUE_SIZE";
     String READ_BYTES = "READ_BYTES";
     String WRITE_BYTES = "WRITE_BYTES";
+
     // Ledger Cache Counters
     String LEDGER_CACHE_HIT = "LEDGER_CACHE_HIT";
     String LEDGER_CACHE_MISS = "LEDGER_CACHE_MISS";
+
     // Compaction/Garbage Collection Related Counters
-    String NUM_MINOR_COMP = "NUM_MINOR_COMP";
-    String NUM_MAJOR_COMP = "NUM_MAJOR_COMP";
+    String ACTIVE_ENTRY_LOG_COUNT = "ACTIVE_ENTRY_LOG_COUNT";
+    String ACTIVE_ENTRY_LOG_SPACE_BYTES = "ACTIVE_ENTRY_LOG_SPACE_BYTES";
+    String RECLAIMED_COMPACTION_SPACE_BYTES = "RECLAIMED_COMPACTION_SPACE_BYTES";
+    String RECLAIMED_DELETION_SPACE_BYTES = "RECLAIMED_DELETION_SPACE_BYTES";
+    String THREAD_RUNTIME = "THREAD_RUNTIME";
+    String MAJOR_COMPACTION_COUNT = "MAJOR_COMPACTION_COUNT";
+    String MINOR_COMPACTION_COUNT = "MINOR_COMPACTION_COUNT";
+    String ACTIVE_LEDGER_COUNT = "ACTIVE_LEDGER_COUNT";
+    String DELETED_LEDGER_COUNT = "DELETED_LEDGER_COUNT";
+
     // Index Related Counters
     String INDEX_INMEM_ILLEGAL_STATE_RESET = "INDEX_INMEM_ILLEGAL_STATE_RESET";
     String INDEX_INMEM_ILLEGAL_STATE_DELETE = "INDEX_INMEM_ILLEGAL_STATE_DELETE";
@@ -136,5 +146,4 @@
     String LD_LEDGER_SCOPE = "ledger";
     String LD_INDEX_SCOPE = "index";
     String LD_WRITABLE_DIRS = "writable_dirs";
-
 }
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
index 588c3d755..e479a2a0f 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
@@ -21,6 +21,14 @@
 
 package org.apache.bookkeeper.bookie;
 
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ACTIVE_ENTRY_LOG_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ACTIVE_ENTRY_LOG_SPACE_BYTES;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.MAJOR_COMPACTION_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.MINOR_COMPACTION_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.RECLAIMED_COMPACTION_SPACE_BYTES;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.RECLAIMED_DELETION_SPACE_BYTES;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.THREAD_RUNTIME;
+
 import com.google.common.annotations.VisibleForTesting;
 import io.netty.util.concurrent.DefaultThreadFactory;
 
@@ -36,9 +44,14 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import java.util.concurrent.atomic.AtomicLong;
 import org.apache.bookkeeper.bookie.GarbageCollector.GarbageCleaner;
 import org.apache.bookkeeper.conf.ServerConfiguration;
 import org.apache.bookkeeper.meta.LedgerManager;
+import org.apache.bookkeeper.stats.Counter;
+import org.apache.bookkeeper.stats.Gauge;
+import org.apache.bookkeeper.stats.OpStatsLogger;
+import org.apache.bookkeeper.stats.StatsLogger;
 import org.apache.bookkeeper.util.MathUtils;
 import org.apache.bookkeeper.util.SafeRunnable;
 import org.slf4j.Logger;
@@ -77,6 +90,18 @@
     // Entry Logger Handle
     final EntryLogger entryLogger;
     final AbstractLogCompactor compactor;
+
+    // Stats loggers for garbage collection operations
+    final StatsLogger statsLogger;
+    private final Counter minorCompactionCounter;
+    private final Counter majorCompactionCounter;
+    private final Counter reclaimedSpaceViaDeletes;
+    private final Counter reclaimedSpaceViaCompaction;
+    private final OpStatsLogger gcThreadRuntime;
+
+    private volatile long totalEntryLogSize;
+    private volatile int numActiveEntryLogs;
+
     final CompactableLedgerStorage ledgerStorage;
 
     // flag to ensure gc thread will not be interrupted during compaction
@@ -110,7 +135,8 @@
      */
     public GarbageCollectorThread(ServerConfiguration conf,
                                   LedgerManager ledgerManager,
-                                  final CompactableLedgerStorage ledgerStorage)
+                                  final CompactableLedgerStorage ledgerStorage,
+                                  StatsLogger statsLogger)
         throws IOException {
         gcExecutor = Executors.newSingleThreadScheduledExecutor(new DefaultThreadFactory("GarbageCollectorThread"));
         this.conf = conf;
@@ -130,7 +156,41 @@ public GarbageCollectorThread(ServerConfiguration conf,
             }
         };
 
-        this.garbageCollector = new ScanAndCompareGarbageCollector(ledgerManager, ledgerStorage, conf);
+        // Stat state initialization
+        this.statsLogger = statsLogger;
+
+        this.minorCompactionCounter = statsLogger.getCounter(MINOR_COMPACTION_COUNT);
+        this.majorCompactionCounter = statsLogger.getCounter(MAJOR_COMPACTION_COUNT);
+        this.reclaimedSpaceViaCompaction = statsLogger.getCounter(RECLAIMED_COMPACTION_SPACE_BYTES);
+        this.reclaimedSpaceViaDeletes = statsLogger.getCounter(RECLAIMED_DELETION_SPACE_BYTES);
+        this.gcThreadRuntime = statsLogger.getOpStatsLogger(THREAD_RUNTIME);
+        this.numActiveEntryLogs = 0;
+        statsLogger.registerGauge(ACTIVE_ENTRY_LOG_COUNT, new Gauge<Integer>() {
+            @Override
+            public Integer getDefaultValue() {
+                return 0;
+            }
+
+            @Override
+            public Integer getSample() {
+                return numActiveEntryLogs;
+            }
+        });
+        this.totalEntryLogSize = 0L;
+        statsLogger.registerGauge(ACTIVE_ENTRY_LOG_SPACE_BYTES, new Gauge<Long>() {
+            @Override
+            public Long getDefaultValue() {
+                return 0L;
+            }
+
+            @Override
+            public Long getSample() {
+                return totalEntryLogSize;
+            }
+        });
+
+        this.garbageCollector = new ScanAndCompareGarbageCollector(ledgerManager, ledgerStorage, conf, statsLogger);
+
         // compaction parameters
         minorCompactionThreshold = conf.getMinorCompactionThreshold();
         minorCompactionInterval = conf.getMinorCompactionInterval() * SECOND;
@@ -267,6 +327,7 @@ public void safeRun() {
     }
 
     public void runWithFlags(boolean force, boolean suspendMajor, boolean suspendMinor) {
+        long threadStart = MathUtils.nowInNano();
         if (force) {
             LOG.info("Garbage collector thread forced to perform GC before expiry of wait time.");
         }
@@ -299,16 +360,17 @@ public void runWithFlags(boolean force, boolean suspendMajor, boolean suspendMin
             lastMajorCompactionTime = MathUtils.now();
             // and also move minor compaction time
             lastMinorCompactionTime = lastMajorCompactionTime;
-            return;
-        }
-
-        if (enableMinorCompaction && (!suspendMinor)
+            majorCompactionCounter.inc();
+        } else if (enableMinorCompaction && (!suspendMinor)
             && (force || curTime - lastMinorCompactionTime > minorCompactionInterval)) {
             // enter minor compaction
             LOG.info("Enter minor compaction, suspendMinor {}", suspendMinor);
             doCompactEntryLogs(minorCompactionThreshold);
             lastMinorCompactionTime = MathUtils.now();
+            minorCompactionCounter.inc();
         }
+        this.gcThreadRuntime.registerSuccessfulEvent(
+                MathUtils.nowInNano() - threadStart, TimeUnit.NANOSECONDS);
     }
 
     /**
@@ -322,6 +384,9 @@ private void doGcLedgers() {
      * Garbage collect those entry loggers which are not associated with any active ledgers.
      */
     private void doGcEntryLogs() {
+        // Get a cumulative count, don't update until complete
+        AtomicLong totalEntryLogSizeAcc = new AtomicLong(0L);
+
         // Loop through all of the entry logs and remove the non-active ledgers.
         entryLogMetaMap.forEach((entryLogId, meta) -> {
             meta.removeLedgerIf((entryLogLedger) -> {
@@ -339,8 +404,14 @@ private void doGcEntryLogs() {
                // We can remove this entry log file now.
                LOG.info("Deleting entryLogId " + entryLogId + " as it has no active ledgers!");
                removeEntryLog(entryLogId);
+               this.reclaimedSpaceViaDeletes.add(meta.getTotalSize());
            }
+
+           totalEntryLogSizeAcc.getAndAdd(meta.getRemainingSize());
         });
+
+        this.totalEntryLogSize = totalEntryLogSizeAcc.get();
+        this.numActiveEntryLogs = entryLogMetaMap.keySet().size();
     }
 
     /**
@@ -368,7 +439,11 @@ void doCompactEntryLogs(double threshold) {
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Compacting entry log {} below threshold {}", meta.getEntryLogId(), threshold);
             }
+
+            long priorRemainingSize = meta.getRemainingSize();
             compactEntryLog(meta);
+            this.reclaimedSpaceViaCompaction.add(meta.getTotalSize() - priorRemainingSize);
+
             if (!running) { // if gc thread is not running, stop compaction
                 return;
             }
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
index bcb5dc508..42a2a33db 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
@@ -102,7 +102,7 @@ public void initialize(ServerConfiguration conf,
         entryLogger = new EntryLogger(conf, ledgerDirsManager, this);
         ledgerCache = new LedgerCacheImpl(conf, activeLedgers,
                 null == indexDirsManager ? ledgerDirsManager : indexDirsManager, statsLogger);
-        gcThread = new GarbageCollectorThread(conf, ledgerManager, this);
+        gcThread = new GarbageCollectorThread(conf, ledgerManager, this, statsLogger.scope("gc"));
         ledgerDirsManager.addLedgerDirsListener(getLedgerDirsListener());
         // Expose Stats
         getOffsetStats = statsLogger.getOpStatsLogger(STORAGE_GET_OFFSET);
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
index b42d302bb..60ed34d2b 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
@@ -21,6 +21,9 @@
 
 package org.apache.bookkeeper.bookie;
 
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ACTIVE_LEDGER_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.DELETED_LEDGER_COUNT;
+
 import com.google.common.collect.Sets;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -41,6 +44,9 @@
 import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
 import org.apache.bookkeeper.net.BookieSocketAddress;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
+import org.apache.bookkeeper.stats.Counter;
+import org.apache.bookkeeper.stats.Gauge;
+import org.apache.bookkeeper.stats.StatsLogger;
 import org.apache.bookkeeper.util.MathUtils;
 import org.apache.bookkeeper.util.ZkUtils;
 import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
@@ -79,9 +85,11 @@
     private long lastOverReplicatedLedgerGcTimeMillis;
     private final String zkLedgersRootPath;
     private final boolean verifyMetadataOnGc;
+    private int activeLedgerCounter;
+    private Counter deletedLedgerCounter;
 
     public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLedgerStorage ledgerStorage,
-            ServerConfiguration conf) throws IOException {
+            ServerConfiguration conf, StatsLogger statsLogger) throws IOException {
         this.ledgerManager = ledgerManager;
         this.ledgerStorage = ledgerStorage;
         this.conf = conf;
@@ -94,7 +102,23 @@ public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLe
         this.zkLedgersRootPath = conf.getZkLedgersRootPath();
         LOG.info("Over Replicated Ledger Deletion : enabled=" + enableGcOverReplicatedLedger + ", interval="
                 + gcOverReplicatedLedgerIntervalMillis);
+
         verifyMetadataOnGc = conf.getVerifyMetadataOnGC();
+
+        this.deletedLedgerCounter = statsLogger.getCounter(DELETED_LEDGER_COUNT);
+
+        this.activeLedgerCounter = 0;
+        statsLogger.registerGauge(ACTIVE_LEDGER_COUNT, new Gauge<Integer>() {
+            @Override
+            public Integer getDefaultValue() {
+                return 0;
+            }
+
+            @Override
+            public Integer getSample() {
+                return activeLedgerCounter;
+            }
+        });
     }
 
     @Override
@@ -109,6 +133,7 @@ public void gc(GarbageCleaner garbageCleaner) {
             // Get a set of all ledgers on the bookie
             NavigableSet<Long> bkActiveLedgers = Sets.newTreeSet(ledgerStorage.getActiveLedgersInRange(0,
                     Long.MAX_VALUE));
+            this.activeLedgerCounter = bkActiveLedgers.size();
 
             long curTime = MathUtils.now();
             boolean checkOverreplicatedLedgers = (enableGcOverReplicatedLedger && curTime
@@ -167,6 +192,7 @@ public void gc(GarbageCleaner garbageCleaner) {
                                 continue;
                             }
                         }
+                        deletedLedgerCounter.inc();
                         garbageCleaner.clean(bkLid);
                     }
                 }
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
index b0257ec03..ccb6533eb 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
@@ -166,7 +166,7 @@ public void initialize(ServerConfiguration conf, LedgerManager ledgerManager, Le
         entryLocationIndex = new EntryLocationIndex(conf, KeyValueStorageRocksDB.factory, baseDir, stats);
 
         entryLogger = new EntryLogger(conf, ledgerDirsManager);
-        gcThread = new GarbageCollectorThread(conf, ledgerManager, this);
+        gcThread = new GarbageCollectorThread(conf, ledgerManager, this, statsLogger);
 
         registerStats();
     }
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
index a6781396e..ee26d8ffe 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
@@ -20,6 +20,13 @@
  */
 package org.apache.bookkeeper.bookie;
 
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ACTIVE_ENTRY_LOG_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ACTIVE_ENTRY_LOG_SPACE_BYTES;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.MAJOR_COMPACTION_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.MINOR_COMPACTION_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.RECLAIMED_COMPACTION_SPACE_BYTES;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.RECLAIMED_DELETION_SPACE_BYTES;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.THREAD_RUNTIME;
 import static org.apache.bookkeeper.bookie.TransactionalEntryLogCompactor.COMPACTED_SUFFIX;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -56,6 +63,7 @@
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.Processor;
 import org.apache.bookkeeper.stats.NullStatsLogger;
 import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.apache.bookkeeper.test.TestStatsProvider;
 import org.apache.bookkeeper.util.DiskChecker;
 import org.apache.bookkeeper.util.HardLink;
 import org.apache.bookkeeper.util.MathUtils;
@@ -282,6 +290,19 @@ public void testMinorCompaction() throws Exception {
         // restart bookies
         restartBookies(baseConf);
 
+        getGCThread().enableForceGC();
+        getGCThread().triggerGC().get();
+        assertTrue(
+                "ACTIVE_ENTRY_LOG_COUNT should have been updated",
+                getStatsProvider(0)
+                        .getGauge("bookie.gc." + ACTIVE_ENTRY_LOG_COUNT)
+                        .getSample().intValue() > 0);
+        assertTrue(
+                "ACTIVE_ENTRY_LOG_SPACE_BYTES should have been updated",
+                getStatsProvider(0)
+                        .getGauge("bookie.gc." + ACTIVE_ENTRY_LOG_SPACE_BYTES)
+                        .getSample().intValue() > 0);
+
         long lastMinorCompactionTime = getGCThread().lastMinorCompactionTime;
         long lastMajorCompactionTime = getGCThread().lastMajorCompactionTime;
         assertFalse(getGCThread().enableMajorCompaction);
@@ -305,9 +326,20 @@ public void testMinorCompaction() throws Exception {
                             + ledgerDirectory, TestUtils.hasLogFiles(ledgerDirectory, true, 0, 1, 2));
         }
 
-        // even entry log files are removed, we still can access entries for ledger1
-        // since those entries has been compacted to new entry log
+        // even though entry log files are removed, we still can access entries for ledger1
+        // since those entries have been compacted to a new entry log
         verifyLedger(lhs[0].getId(), 0, lhs[0].getLastAddConfirmed());
+
+        assertTrue(
+                "RECLAIMED_COMPACTION_SPACE_BYTES should have been updated",
+                getStatsProvider(0)
+                        .getCounter("bookie.gc." + RECLAIMED_COMPACTION_SPACE_BYTES)
+                        .get().intValue() > 0);
+        assertTrue(
+                "RECLAIMED_DELETION_SPACE_BYTES should have been updated",
+                getStatsProvider(0)
+                        .getCounter("bookie.gc." + RECLAIMED_DELETION_SPACE_BYTES)
+                        .get().intValue() > 0);
     }
 
     @Test
@@ -838,6 +870,7 @@ public void checkpointComplete(Checkpoint checkPoint, boolean compact)
             Bookie.checkDirectoryStructure(dir);
         }
         InterleavedLedgerStorage storage = new InterleavedLedgerStorage();
+        TestStatsProvider stats = new TestStatsProvider();
         storage.initialize(
             conf,
             LedgerManagerFactory.newLedgerManagerFactory(conf, zkc).newLedgerManager(),
@@ -845,30 +878,66 @@ public void checkpointComplete(Checkpoint checkPoint, boolean compact)
             dirManager,
             cp,
             Checkpointer.NULL,
-            NullStatsLogger.INSTANCE);
+            stats.getStatsLogger("storage"));
         storage.start();
 
-        // test suspend Major GC.
+        int majorCompactions = stats.getCounter("storage.gc." + MAJOR_COMPACTION_COUNT).get().intValue();
+        int minorCompactions = stats.getCounter("storage.gc." + MINOR_COMPACTION_COUNT).get().intValue();
         Thread.sleep(conf.getMajorCompactionInterval() * 1000
-                   + conf.getGcWaitTime());
+                + conf.getGcWaitTime());
+        assertTrue(
+                "Major compaction should have happened",
+                stats.getCounter("storage.gc." + MAJOR_COMPACTION_COUNT).get() > majorCompactions);
+
+        // test suspend Major GC.
         storage.gcThread.suspendMajorGC();
+
         Thread.sleep(1000);
         long startTime = MathUtils.now();
+        majorCompactions = stats.getCounter("storage.gc." + MAJOR_COMPACTION_COUNT).get().intValue();
         Thread.sleep(conf.getMajorCompactionInterval() * 1000
                    + conf.getGcWaitTime());
-        assertTrue("major compaction triggered while set suspend",
+        assertTrue("major compaction triggered while suspended",
                 storage.gcThread.lastMajorCompactionTime < startTime);
+        assertTrue("major compaction triggered while suspended",
+                stats.getCounter("storage.gc." + MAJOR_COMPACTION_COUNT).get() == majorCompactions);
+
+        // test suspend Major GC.
+        Thread.sleep(conf.getMinorCompactionInterval() * 1000
+                + conf.getGcWaitTime());
+        assertTrue(
+                "Minor compaction should have happened",
+                stats.getCounter("storage.gc." + MINOR_COMPACTION_COUNT).get() > minorCompactions);
 
         // test suspend Minor GC.
         storage.gcThread.suspendMinorGC();
+
         Thread.sleep(1000);
         startTime = MathUtils.now();
+        minorCompactions = stats.getCounter("storage.gc." + MINOR_COMPACTION_COUNT).get().intValue();
         Thread.sleep(conf.getMajorCompactionInterval() * 1000
                    + conf.getGcWaitTime());
-        assertTrue("minor compaction triggered while set suspend",
+        assertTrue("minor compaction triggered while suspended",
                 storage.gcThread.lastMinorCompactionTime < startTime);
+        assertTrue("minor compaction triggered while suspended",
+                stats.getCounter("storage.gc." + MINOR_COMPACTION_COUNT).get() == minorCompactions);
+
+        // test resume
         storage.gcThread.resumeMinorGC();
         storage.gcThread.resumeMajorGC();
+
+        Thread.sleep((conf.getMajorCompactionInterval() + conf.getMinorCompactionInterval()) * 1000
+                + (conf.getGcWaitTime() * 2));
+        assertTrue(
+                "Major compaction should have happened",
+                stats.getCounter("storage.gc." + MAJOR_COMPACTION_COUNT).get() > majorCompactions);
+        assertTrue(
+                "Minor compaction should have happened",
+                stats.getCounter("storage.gc." + MINOR_COMPACTION_COUNT).get() > minorCompactions);
+        assertTrue(
+                "gcThreadRunttime should be non-zero",
+                stats.getOpStatsLogger("storage.gc." + THREAD_RUNTIME).getSuccessCount() > 0);
+
     }
 
     @Test
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestGcOverreplicatedLedger.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestGcOverreplicatedLedger.java
index 116fab81a..fd2bf5540 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestGcOverreplicatedLedger.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestGcOverreplicatedLedger.java
@@ -47,6 +47,7 @@
 import org.apache.bookkeeper.net.BookieSocketAddress;
 import org.apache.bookkeeper.proto.BookieServer;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
+import org.apache.bookkeeper.stats.NullStatsLogger;
 import org.apache.bookkeeper.util.SnapshotMap;
 import org.apache.zookeeper.ZooDefs;
 import org.junit.Assert;
@@ -108,7 +109,7 @@ public void operationComplete(int rc, LedgerMetadata result) {
 
         final CompactableLedgerStorage mockLedgerStorage = new MockLedgerStorage();
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(ledgerManager, mockLedgerStorage,
-                bkConf);
+                bkConf, NullStatsLogger.INSTANCE);
         Thread.sleep(bkConf.getGcOverreplicatedLedgerWaitTimeMillis() + 1);
         garbageCollector.gc(new GarbageCleaner() {
 
@@ -159,7 +160,7 @@ public void operationComplete(int rc, LedgerMetadata result) {
 
         final CompactableLedgerStorage mockLedgerStorage = new MockLedgerStorage();
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(ledgerManager, mockLedgerStorage,
-                bkConf);
+                bkConf, NullStatsLogger.INSTANCE);
         Thread.sleep(bkConf.getGcOverreplicatedLedgerWaitTimeMillis() + 1);
         garbageCollector.gc(new GarbageCleaner() {
 
@@ -209,7 +210,7 @@ public void operationComplete(int rc, LedgerMetadata result) {
 
         final CompactableLedgerStorage mockLedgerStorage = new MockLedgerStorage();
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(ledgerManager, mockLedgerStorage,
-                bkConf);
+                bkConf, NullStatsLogger.INSTANCE);
         Thread.sleep(bkConf.getGcOverreplicatedLedgerWaitTimeMillis() + 1);
         garbageCollector.gc(new GarbageCleaner() {
 
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
index 730490bae..123158000 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java
@@ -21,11 +21,14 @@
 
 package org.apache.bookkeeper.meta;
 
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ACTIVE_LEDGER_COUNT;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.DELETED_LEDGER_COUNT;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import io.netty.buffer.ByteBuf;
 import java.io.IOException;
@@ -64,7 +67,9 @@
 import org.apache.bookkeeper.meta.LedgerManager.LedgerRange;
 import org.apache.bookkeeper.meta.LedgerManager.LedgerRangeIterator;
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
+import org.apache.bookkeeper.stats.NullStatsLogger;
 import org.apache.bookkeeper.stats.StatsLogger;
+import org.apache.bookkeeper.test.TestStatsProvider;
 import org.apache.bookkeeper.versioning.Version;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -180,8 +185,9 @@ public void operationComplete(int rc, Void result) {
         final CountDownLatch createLatch = new CountDownLatch(1);
         final CountDownLatch endLatch = new CountDownLatch(2);
         final CompactableLedgerStorage mockLedgerStorage = new MockLedgerStorage();
+        TestStatsProvider stats = new TestStatsProvider();
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(getLedgerManager(),
-                mockLedgerStorage, baseConf);
+                mockLedgerStorage, baseConf, stats.getStatsLogger("gc"));
         Thread gcThread = new Thread() {
             @Override
             public void run() {
@@ -241,6 +247,12 @@ public void run() {
         for (Long ledger : createdLedgers) {
             assertTrue(activeLedgers.containsKey(ledger));
         }
+        assertTrue(
+                "Wrong DELETED_LEDGER_COUNT",
+                stats.getCounter("gc." + DELETED_LEDGER_COUNT).get() == removedLedgers.size());
+        assertTrue(
+                "Wrong ACTIVE_LEDGER_COUNT",
+                stats.getGauge("gc." + ACTIVE_LEDGER_COUNT).getSample().intValue() == createdLedgers.size());
     }
 
     @Test
@@ -251,30 +263,58 @@ public void testGcLedgersOutsideRange() throws Exception {
 
         createLedgers(numLedgers, createdLedgers);
 
+        MockLedgerStorage mockLedgerStorage = new MockLedgerStorage();
+        TestStatsProvider stats = new TestStatsProvider();
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(getLedgerManager(),
-                new MockLedgerStorage(), baseConf);
+                mockLedgerStorage, baseConf, stats.getStatsLogger("gc"));
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
                 @Override
                 public void clean(long ledgerId) {
                     LOG.info("Cleaned {}", ledgerId);
                     cleaned.add(ledgerId);
+                    try {
+                        mockLedgerStorage.deleteLedger(ledgerId);
+                    } catch (IOException e) {
+                        e.printStackTrace();
+                        fail("Exception from deleteLedger");
+                    }
                 }
             };
 
         garbageCollector.gc(cleaner);
         assertNull("Should have cleaned nothing", cleaned.poll());
+        assertTrue(
+                "Wrong DELETED_LEDGER_COUNT",
+                stats.getCounter("gc." + DELETED_LEDGER_COUNT).get() == 0);
+        assertTrue(
+                "Wrong ACTIVE_LEDGER_COUNT",
+                stats.getGauge(
+                        "gc." + ACTIVE_LEDGER_COUNT).getSample().intValue() == numLedgers);
 
         long last = createdLedgers.last();
         removeLedger(last);
         garbageCollector.gc(cleaner);
         assertNotNull("Should have cleaned something", cleaned.peek());
         assertEquals("Should have cleaned last ledger" + last, (long) last, (long) cleaned.poll());
+        assertTrue(
+                "Wrong DELETED_LEDGER_COUNT",
+                stats.getCounter("gc." + DELETED_LEDGER_COUNT).get() == 1);
 
         long first = createdLedgers.first();
         removeLedger(first);
         garbageCollector.gc(cleaner);
         assertNotNull("Should have cleaned something", cleaned.peek());
         assertEquals("Should have cleaned first ledger" + first, (long) first, (long) cleaned.poll());
+        assertTrue(
+                "Wrong DELETED_LEDGER_COUNT",
+                stats.getCounter("gc." + DELETED_LEDGER_COUNT).get() == 2);
+
+        garbageCollector.gc(cleaner);
+        assertTrue(
+                "Wrong ACTIVE_LEDGER_COUNT",
+                stats.getGauge(
+                        "gc." + ACTIVE_LEDGER_COUNT).getSample().intValue() == (numLedgers - 2));
+
     }
 
     @Test
@@ -288,7 +328,7 @@ public void testGcLedgersNotLast() throws Exception {
         createLedgers(numLedgers, createdLedgers);
 
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(getLedgerManager(),
-                new MockLedgerStorage(), baseConf);
+                new MockLedgerStorage(), baseConf, NullStatsLogger.INSTANCE);
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
                 @Override
                 public void clean(long ledgerId) {
@@ -328,7 +368,7 @@ public void testGcLedgersWithNoLedgers() throws Exception {
         // no ledger created
 
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(getLedgerManager(),
-                new MockLedgerStorage(), baseConf);
+                new MockLedgerStorage(), baseConf, NullStatsLogger.INSTANCE);
         AtomicBoolean cleanerCalled = new AtomicBoolean(false);
 
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
@@ -358,7 +398,7 @@ public void testGcLedgersWithLedgersInSameLedgerRange() throws Exception {
         createLedgers(numLedgers, createdLedgers);
 
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(getLedgerManager(),
-                new MockLedgerStorage(), baseConf);
+                new MockLedgerStorage(), baseConf, NullStatsLogger.INSTANCE);
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
             @Override
             public void clean(long ledgerId) {
@@ -420,7 +460,7 @@ public boolean hasNext() throws IOException {
         };
 
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(mockLedgerManager,
-                new MockLedgerStorage(), baseConf);
+                new MockLedgerStorage(), baseConf, NullStatsLogger.INSTANCE);
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
             @Override
             public void clean(long ledgerId) {
@@ -467,7 +507,7 @@ public void readLedgerMetadata(long ledgerId, GenericCallback<LedgerMetadata> re
         };
 
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(mockLedgerManager,
-                new MockLedgerStorage(), baseConf);
+                new MockLedgerStorage(), baseConf, NullStatsLogger.INSTANCE);
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
             @Override
             public void clean(long ledgerId) {
@@ -509,7 +549,7 @@ public void readLedgerMetadata(long ledgerId, GenericCallback<LedgerMetadata> re
         };
 
         final GarbageCollector garbageCollector = new ScanAndCompareGarbageCollector(mockLedgerManager,
-                new MockLedgerStorage(), baseConf);
+                new MockLedgerStorage(), baseConf, NullStatsLogger.INSTANCE);
         GarbageCollector.GarbageCleaner cleaner = new GarbageCollector.GarbageCleaner() {
             @Override
             public void clean(long ledgerId) {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services