GEODE-467: fix race in off-heap memory monitor tests Fixed the hang in testHeapLRUWithOverflowToDisk. It turned out that in many cases the off-heap memory usage listener thread was never terminating. So when this test waited for it to terminate it hung.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/c9a7f102 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/c9a7f102 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/c9a7f102 Branch: refs/heads/develop Commit: c9a7f1027d234fe26879b31d979c12214095bb5d Parents: 272729b Author: Darrel Schneider <[email protected]> Authored: Wed Oct 28 14:55:24 2015 -0700 Committer: Darrel Schneider <[email protected]> Committed: Wed Oct 28 15:16:02 2015 -0700 ---------------------------------------------------------------------- .../cache/control/OffHeapMemoryMonitor.java | 25 +++++++++++++++----- .../cache/OffHeapEvictionDUnitTest.java | 2 +- ...rtitionedRegionOffHeapEvictionDUnitTest.java | 4 +++- .../control/MemoryMonitorOffHeapJUnitTest.java | 2 +- 4 files changed, 24 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c9a7f102/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/control/OffHeapMemoryMonitor.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/control/OffHeapMemoryMonitor.java b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/control/OffHeapMemoryMonitor.java index c4e9df6..bbdb27a 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/control/OffHeapMemoryMonitor.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/control/OffHeapMemoryMonitor.java @@ -59,6 +59,7 @@ public class OffHeapMemoryMonitor implements ResourceMonitor, MemoryUsageListene private boolean hasEvictionThreshold = false; private OffHeapMemoryUsageListener offHeapMemoryUsageListener; + private Thread memoryListenerThread; private final InternalResourceManager resourceManager; private final ResourceAdvisor resourceAdvisor; @@ -93,11 +94,12 @@ public class OffHeapMemoryMonitor implements ResourceMonitor, MemoryUsageListene this.offHeapMemoryUsageListener = new OffHeapMemoryUsageListener(getBytesUsed()); ThreadGroup group = LoggingThreadGroup.createThreadGroup("OffHeapMemoryMonitor Threads", logger); - Thread memoryListenerThread = new Thread(group, this.offHeapMemoryUsageListener); - memoryListenerThread.setName(memoryListenerThread.getName() + " OffHeapMemoryListener"); - memoryListenerThread.setPriority(Thread.MAX_PRIORITY); - memoryListenerThread.setDaemon(true); - memoryListenerThread.start(); + Thread t = new Thread(group, this.offHeapMemoryUsageListener); + t.setName(t.getName() + " OffHeapMemoryListener"); + t.setPriority(Thread.MAX_PRIORITY); + t.setDaemon(true); + t.start(); + this.memoryListenerThread = t; this.memoryAllocator.addMemoryUsageListener(this); @@ -110,6 +112,9 @@ public class OffHeapMemoryMonitor implements ResourceMonitor, MemoryUsageListene */ @Override public void stopMonitoring() { + stopMonitoring(false); + } + public void stopMonitoring(boolean waitForThread) { synchronized (this) { if (!this.started) { return; @@ -122,6 +127,14 @@ public class OffHeapMemoryMonitor implements ResourceMonitor, MemoryUsageListene this.offHeapMemoryUsageListener.notifyAll(); } + if (waitForThread && this.memoryListenerThread != null) { + try { + this.memoryListenerThread.join(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + this.memoryListenerThread = null; this.started = false; } } @@ -526,7 +539,7 @@ public class OffHeapMemoryMonitor implements ResourceMonitor, MemoryUsageListene lastOffHeapMemoryUsed = this.offHeapMemoryUsed; break; } - } while (true); + } while (!this.stopRequested); } catch (InterruptedException iex) { getLogWriter().warning("OffHeapMemoryUsageListener was interrupted " + this); this.stopRequested = true; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c9a7f102/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/OffHeapEvictionDUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/OffHeapEvictionDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/OffHeapEvictionDUnitTest.java index 57cdfae..386f8ce 100644 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/OffHeapEvictionDUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/OffHeapEvictionDUnitTest.java @@ -78,7 +78,7 @@ public class OffHeapEvictionDUnitTest extends EvictionDUnitDisabledTest { getLogWriter().info("cache= " + cache); getLogWriter().info("cache closed= " + cache.isClosed()); cache.getResourceManager().setEvictionOffHeapPercentage(85); - ((GemFireCacheImpl) cache).getResourceManager().getOffHeapMonitor().stopMonitoring(); + ((GemFireCacheImpl) cache).getResourceManager().getOffHeapMonitor().stopMonitoring(true); getLogWriter().info("eviction= "+cache.getResourceManager().getEvictionOffHeapPercentage()); getLogWriter().info("critical= "+cache.getResourceManager().getCriticalOffHeapPercentage()); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c9a7f102/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionOffHeapEvictionDUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionOffHeapEvictionDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionOffHeapEvictionDUnitTest.java index f07c5b1..672b8a7 100644 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionOffHeapEvictionDUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/PartitionedRegionOffHeapEvictionDUnitTest.java @@ -80,16 +80,18 @@ public class PartitionedRegionOffHeapEvictionDUnitTest extends return ((GemFireCacheImpl)region.getRegionService()).getOffHeapEvictor(); } + @Override protected void raiseFakeNotification() { ((GemFireCacheImpl) getCache()).getOffHeapEvictor().testAbortAfterLoopCount = 1; setEvictionPercentage(85); OffHeapMemoryMonitor ohmm = ((GemFireCacheImpl) getCache()).getResourceManager().getOffHeapMonitor(); - ohmm.stopMonitoring(); + ohmm.stopMonitoring(true); ohmm.updateStateAndSendEvent(94371840); } + @Override protected void cleanUpAfterFakeNotification() { ((GemFireCacheImpl) getCache()).getOffHeapEvictor().testAbortAfterLoopCount = Integer.MAX_VALUE; } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c9a7f102/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/control/MemoryMonitorOffHeapJUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/control/MemoryMonitorOffHeapJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/control/MemoryMonitorOffHeapJUnitTest.java index d7a875c..bf8be0a 100644 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/control/MemoryMonitorOffHeapJUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/control/MemoryMonitorOffHeapJUnitTest.java @@ -92,7 +92,7 @@ public class MemoryMonitorOffHeapJUnitTest { monitor.setEvictionThreshold(50.0f); monitor.setCriticalThreshold(75.0f); - monitor.stopMonitoring(); + monitor.stopMonitoring(true); assertEquals(524288, internalManager.getStats().getOffHeapEvictionThreshold()); assertEquals(786432, internalManager.getStats().getOffHeapCriticalThreshold());
