[ 
https://issues.apache.org/jira/browse/GEODE-527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16329615#comment-16329615
 ] 

ASF GitHub Bot commented on GEODE-527:
--------------------------------------

nreich closed pull request #1296: GEODE-527: Fix race condition that caused 
sporadic test failure
URL: https://github.com/apache/geode/pull/1296
 
 
   

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/geode-core/src/test/java/org/apache/geode/internal/cache/OplogJUnitTest.java 
b/geode-core/src/test/java/org/apache/geode/internal/cache/OplogJUnitTest.java
index 09c24cb593..b2592770e1 100755
--- 
a/geode-core/src/test/java/org/apache/geode/internal/cache/OplogJUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/internal/cache/OplogJUnitTest.java
@@ -14,7 +14,12 @@
  */
 package org.apache.geode.internal.cache;
 
-import static org.junit.Assert.*;
+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 java.io.File;
 import java.io.FileInputStream;
@@ -24,8 +29,10 @@
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.IntStream;
 
 import org.apache.commons.io.FileUtils;
@@ -51,7 +58,6 @@
 import org.apache.geode.internal.cache.entries.AbstractDiskLRURegionEntry;
 import org.apache.geode.internal.cache.entries.DiskEntry;
 import org.apache.geode.test.dunit.ThreadUtils;
-import org.apache.geode.test.junit.categories.FlakyTest;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 /**
@@ -1571,8 +1577,6 @@ public void afterHavingCompacted() {
   /**
    * Tests reduction in size of disk stats when the oplog is rolled.
    */
-  @Category(FlakyTest.class) // GEODE-527: jvm sizing sensitive, 
non-thread-safe test hooks, time
-                             // sensitive
   @Test
   public void testStatsSizeReductionOnRolling() throws Exception {
     final int MAX_OPLOG_SIZE = 500 * 2;
@@ -1586,10 +1590,6 @@ public void testStatsSizeReductionOnRolling() throws 
Exception {
     final byte[] val = new byte[333];
     region = DiskRegionHelperFactory.getSyncPersistOnlyRegion(cache, 
diskProps, Scope.LOCAL);
     final DiskRegion dr = ((LocalRegion) region).getDiskRegion();
-    final Object lock = new Object();
-    final boolean[] exceptionOccurred = new boolean[] {true};
-    final boolean[] okToExit = new boolean[] {false};
-    final boolean[] switchExpected = new boolean[] {false};
 
     // calculate sizes
     final int extra_byte_num_per_entry =
@@ -1601,63 +1601,12 @@ public void testStatsSizeReductionOnRolling() throws 
Exception {
     final int tombstone_key2 =
         
DiskOfflineCompactionJUnitTest.getSize4TombstoneWithKey(extra_byte_num_per_entry,
 "key2");
 
+    CountDownLatch putsCompleted = new CountDownLatch(1);
     // TODO: move static methods from DiskOfflineCompactionJUnitTest to shared 
util class
+    StatSizeTestCacheObserverAdapter testObserver = new 
StatSizeTestCacheObserverAdapter(dr,
+        key3_size, tombstone_key1, tombstone_key2, putsCompleted);
+    CacheObserver old = CacheObserverHolder.setInstance(testObserver);
 
-    CacheObserver old = CacheObserverHolder.setInstance(new 
CacheObserverAdapter() {
-      private long before = -1;
-      private DirectoryHolder dh = null;
-      private long oplogsSize = 0;
-
-      @Override
-      public void beforeSwitchingOplog() {
-        cache.getLogger().info("beforeSwitchingOplog");
-        if (!switchExpected[0]) {
-          fail("unexpected oplog switch");
-        }
-        if (before == -1) {
-          // only want to call this once; before the 1st oplog destroy
-          this.dh = dr.getNextDir();
-          this.before = this.dh.getDirStatsDiskSpaceUsage();
-        }
-      }
-
-      @Override
-      public void beforeDeletingCompactedOplog(Oplog oplog) {
-        cache.getLogger().info("beforeDeletingCompactedOplog");
-        oplogsSize += oplog.getOplogSize();
-      }
-
-      @Override
-      public void afterHavingCompacted() {
-        cache.getLogger().info("afterHavingCompacted");
-        if (before > -1) {
-          synchronized (lock) {
-            okToExit[0] = true;
-            long after = this.dh.getDirStatsDiskSpaceUsage();
-            // after compaction, in _2.crf, key3 is an create-entry,
-            // key1 and key2 are tombstones.
-            // _2.drf contained a rvvgc with drMap.size()==1
-            int expected_drf_size = Oplog.OPLOG_DISK_STORE_REC_SIZE + 
Oplog.OPLOG_MAGIC_SEQ_REC_SIZE
-                + Oplog.OPLOG_GEMFIRE_VERSION_REC_SIZE
-                + DiskOfflineCompactionJUnitTest.getRVVSize(1, new int[] {0}, 
true);
-            int expected_crf_size = Oplog.OPLOG_DISK_STORE_REC_SIZE + 
Oplog.OPLOG_MAGIC_SEQ_REC_SIZE
-                + Oplog.OPLOG_GEMFIRE_VERSION_REC_SIZE
-                + DiskOfflineCompactionJUnitTest.getRVVSize(1, new int[] {1}, 
false)
-                + Oplog.OPLOG_NEW_ENTRY_BASE_REC_SIZE + key3_size + 
tombstone_key1 + tombstone_key2;
-            int oplog_2_size = expected_drf_size + expected_crf_size;
-            if (after != oplog_2_size) {
-              cache.getLogger().info(
-                  "test failed before=" + before + " after=" + after + " 
oplogsSize=" + oplogsSize);
-              exceptionOccurred[0] = true;
-            } else {
-              exceptionOccurred[0] = false;
-            }
-            LocalRegion.ISSUE_CALLBACKS_TO_CACHE_OBSERVER = false;
-            lock.notify();
-          }
-        }
-      }
-    });
     try {
 
       LocalRegion.ISSUE_CALLBACKS_TO_CACHE_OBSERVER = true;
@@ -1674,17 +1623,13 @@ public void afterHavingCompacted() {
       region.remove("key2");
 
       // This put will cause a switch as max-oplog size (900) will be exceeded 
(999)
-      switchExpected[0] = true;
+      testObserver.setSwitchExpected();
       cache.getLogger().info("putting key3");
       region.put("key3", val);
+      putsCompleted.countDown();
       cache.getLogger().info("waiting for compaction");
-      synchronized (lock) {
-        if (!okToExit[0]) {
-          lock.wait(9000);
-          assertTrue(okToExit[0]);
-        }
-        assertFalse(exceptionOccurred[0]);
-      }
+      Awaitility.await().atMost(9, TimeUnit.SECONDS).until(() -> 
testObserver.hasCompacted());
+      assertFalse(testObserver.exceptionOccured());
 
       region.close();
     } finally {
@@ -1992,4 +1937,92 @@ private long oplogSize() {
   private int getDSID(LocalRegion lr) {
     return lr.getDistributionManager().getDistributedSystemId();
   }
+
+  private class StatSizeTestCacheObserverAdapter extends CacheObserverAdapter {
+    private final AtomicBoolean switchExpected = new AtomicBoolean(false);
+    private final DiskRegion dr;
+    private final AtomicBoolean hasCompacted = new AtomicBoolean(false);
+    private final int key3Size;
+    private final int tombstoneKey1;
+    private final int tombstoneKey2;
+    private final AtomicBoolean exceptionOccurred = new AtomicBoolean(true);
+    private volatile long spaceUsageBefore = -1;
+    private DirectoryHolder dh;
+    private final AtomicLong oplogsSize = new AtomicLong();
+    private final CountDownLatch putsCompleted;
+
+    StatSizeTestCacheObserverAdapter(DiskRegion dr, int key3Size, int 
tombstoneKey1,
+        int tombstoneKey2, CountDownLatch putsCompleted) {
+      this.dr = dr;
+      this.key3Size = key3Size;
+      this.tombstoneKey1 = tombstoneKey1;
+      this.tombstoneKey2 = tombstoneKey2;
+      this.putsCompleted = putsCompleted;
+    }
+
+    @Override
+    public void beforeSwitchingOplog() {
+      cache.getLogger().info("beforeSwitchingOplog");
+      if (!switchExpected.get()) {
+        fail("unexpected oplog switch");
+      }
+      if (spaceUsageBefore == -1) {
+        // only want to call this once; before the 1st oplog destroy
+        this.dh = dr.getNextDir();
+        this.spaceUsageBefore = this.dh.getDirStatsDiskSpaceUsage();
+      }
+    }
+
+    @Override
+    public void beforeDeletingCompactedOplog(Oplog oplog) {
+      cache.getLogger().info("beforeDeletingCompactedOplog");
+      oplogsSize.addAndGet(oplog.getOplogSize());
+    }
+
+    @Override
+    public void afterHavingCompacted() {
+      try {
+        putsCompleted.await(10, TimeUnit.SECONDS);
+      } catch (InterruptedException e) {
+        exceptionOccurred.set(true);
+        throw new RuntimeException(e);
+      }
+      cache.getLogger().info("afterHavingCompacted");
+      if (spaceUsageBefore > -1) {
+        hasCompacted.set(true);
+        long after = this.dh.getDirStatsDiskSpaceUsage();
+        // after compaction, in _2.crf, key3 is an create-entry,
+        // key1 and key2 are tombstones.
+        // _2.drf contained a rvvgc with drMap.size()==1
+        int expectedDrfSize = Oplog.OPLOG_DISK_STORE_REC_SIZE + 
Oplog.OPLOG_MAGIC_SEQ_REC_SIZE
+            + Oplog.OPLOG_GEMFIRE_VERSION_REC_SIZE
+            + DiskOfflineCompactionJUnitTest.getRVVSize(1, new int[] {0}, 
true);
+        int expectedCrfSize = Oplog.OPLOG_DISK_STORE_REC_SIZE + 
Oplog.OPLOG_MAGIC_SEQ_REC_SIZE
+            + Oplog.OPLOG_GEMFIRE_VERSION_REC_SIZE
+            + DiskOfflineCompactionJUnitTest.getRVVSize(1, new int[] {1}, 
false)
+            + Oplog.OPLOG_NEW_ENTRY_BASE_REC_SIZE + key3Size + tombstoneKey1 + 
tombstoneKey2;
+        int oplog2Size = expectedDrfSize + expectedCrfSize;
+        if (after != oplog2Size) {
+          cache.getLogger().info("test failed before=" + spaceUsageBefore + " 
after=" + after
+              + " expected=" + oplog2Size);
+          exceptionOccurred.set(true);
+        } else {
+          exceptionOccurred.set(false);
+        }
+        LocalRegion.ISSUE_CALLBACKS_TO_CACHE_OBSERVER = false;
+      }
+    }
+
+    boolean hasCompacted() {
+      return hasCompacted.get();
+    }
+
+    boolean exceptionOccured() {
+      return exceptionOccurred.get();
+    }
+
+    void setSwitchExpected() {
+      switchExpected.set(true);
+    }
+  }
 }


 

----------------------------------------------------------------
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:
[email protected]


> CI failure: OplogJUnitTest.testStatsSizeReductionOnRolling (IntegrationTest)
> ----------------------------------------------------------------------------
>
>                 Key: GEODE-527
>                 URL: https://issues.apache.org/jira/browse/GEODE-527
>             Project: Geode
>          Issue Type: Bug
>          Components: persistence
>            Reporter: Kirk Lund
>            Assignee: Nick Reich
>            Priority: Major
>              Labels: CI, Flaky, IntegrationTest, pull-request-available
>         Attachments: sysout.txt
>
>
> {noformat}
> java.lang.AssertionError
>       at org.junit.Assert.fail(Assert.java:86)
>       at org.junit.Assert.assertTrue(Assert.java:41)
>       at org.junit.Assert.assertFalse(Assert.java:64)
>       at org.junit.Assert.assertFalse(Assert.java:74)
>       at 
> com.gemstone.gemfire.internal.cache.OplogJUnitTest.testStatsSizeReductionOnRolling(OplogJUnitTest.java:3564)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:497)
>       at 
> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
>       at 
> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
>       at 
> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
>       at 
> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
>       at 
> org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
>       at 
> org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
>       at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
>       at org.junit.rules.RunRules.evaluate(RunRules.java:20)
>       at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
>       at 
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
>       at 
> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
>       at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
>       at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
>       at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
>       at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
>       at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
>       at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
>       at 
> org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:105)
>       at 
> org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:56)
>       at 
> org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:64)
>       at 
> org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:50)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:497)
>       at 
> org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
>       at 
> org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
>       at 
> org.gradle.messaging.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
>       at 
> org.gradle.messaging.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
>       at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
>       at 
> org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:106)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:497)
>       at 
> org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
>       at 
> org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
>       at 
> org.gradle.messaging.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:360)
>       at 
> org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:54)
>       at 
> org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:40)
>       at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
>       at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
>       at java.lang.Thread.run(Thread.java:745)
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to