GEODE-11: Fixing some races in Lucene Query DUnits The tests in LuceneQueryBase had a couple of race conditions. The first test didn't actually wait for entries to be flushed, so it could run the query before the flush happens.
The wait for flush test had a test hook with a 1 second pause, but that may not be long enough depending on what happens on the system. I changed the test to pause the sender instead, for a deterministic test. Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/4a97daf6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/4a97daf6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/4a97daf6 Branch: refs/heads/feature/GEODE-835 Commit: 4a97daf63627f3e1d464996167f3dbbdffe29ac2 Parents: 8d1fa0c Author: Dan Smith <upthewatersp...@apache.org> Authored: Wed May 18 16:40:28 2016 -0700 Committer: Dan Smith <upthewatersp...@apache.org> Committed: Thu May 19 17:02:05 2016 -0700 ---------------------------------------------------------------------- .../lucene/internal/LuceneEventListener.java | 9 ---- ...IndexCreationPersistenceIntegrationTest.java | 7 +-- .../gemfire/cache/lucene/LuceneQueriesBase.java | 57 +++++--------------- .../cache/lucene/LuceneQueriesPRBase.java | 2 + .../lucene/LuceneQueriesPeerPRDUnitTest.java | 1 + .../LuceneQueriesPeerPROverflowDUnitTest.java | 2 + .../cache/lucene/test/LuceneTestUtilities.java | 11 ++++ 7 files changed, 31 insertions(+), 58 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java b/geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java index 2dae4ee..ca8077d 100644 --- a/geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java +++ b/geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneEventListener.java @@ -72,10 +72,6 @@ public class LuceneEventListener implements AsyncEventListener { IndexRepository repository = repositoryManager.getRepository(region, key, callbackArgument); Operation op = event.getOperation(); - - if (testHook != null) { - testHook.doTestHook("FOUND_AND_BEFORE_PROCESSING_A_EVENT"); - } if (op.isCreate()) { repository.create(key, event.getDeserializedValue()); @@ -102,9 +98,4 @@ public class LuceneEventListener implements AsyncEventListener { DefaultQuery.setPdxReadSerialized(false); } } - - public interface TestHook { - public void doTestHook(String spot); - } - public static TestHook testHook; } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java index 23983cb..d6bf116 100644 --- a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java +++ b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexCreationPersistenceIntegrationTest.java @@ -22,23 +22,19 @@ import static com.gemstone.gemfire.cache.lucene.test.LuceneTestUtilities.*; import static org.junit.Assert.*; import java.io.File; -import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import com.gemstone.gemfire.cache.Region; import com.gemstone.gemfire.cache.RegionShortcut; import com.gemstone.gemfire.cache.asyncqueue.AsyncEventQueue; -import com.gemstone.gemfire.cache.asyncqueue.internal.AsyncEventQueueImpl; import com.gemstone.gemfire.cache.lucene.test.LuceneTestUtilities; import com.gemstone.gemfire.cache.lucene.test.TestObject; import com.gemstone.gemfire.internal.cache.GemFireCacheImpl; import com.gemstone.gemfire.internal.cache.LocalRegion; import com.gemstone.gemfire.test.junit.categories.IntegrationTest; import com.gemstone.gemfire.test.junit.rules.DiskDirRule; -import com.jayway.awaitility.Awaitility; import org.apache.lucene.queryparser.classic.ParseException; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -101,8 +97,7 @@ public class LuceneIndexCreationPersistenceIntegrationTest extends LuceneIntegra Region dataRegion = cache.createRegionFactory(RegionShortcut.PARTITION_PERSISTENT) .create(REGION_NAME); //Pause the sender so that the entry stays in the queue - final AsyncEventQueueImpl queue = (AsyncEventQueueImpl) getIndexQueue(cache); - queue.getSender().pause(); + pauseSender(cache); dataRegion.put("A", new TestObject()); cache.close(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java index c7567f3..821be17 100644 --- a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java +++ b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesBase.java @@ -18,6 +18,7 @@ */ package com.gemstone.gemfire.cache.lucene; +import static com.gemstone.gemfire.cache.lucene.test.LuceneTestUtilities.*; import static org.junit.Assert.*; import java.io.Serializable; @@ -48,8 +49,6 @@ import org.junit.Test; */ public abstract class LuceneQueriesBase extends JUnit4CacheTestCase { - protected static final String INDEX_NAME = "index"; - protected static final String REGION_NAME = "index"; private static final long serialVersionUID = 1L; protected VM dataStore1; protected VM dataStore2; @@ -78,6 +77,7 @@ public abstract class LuceneQueriesBase extends JUnit4CacheTestCase { accessor.invoke(() -> initAccessor(createIndex)); putDataInRegion(accessor); + assertTrue(waitForFlushBeforeExecuteTextSearch(accessor, 60000)); executeTextSearch(accessor); } @@ -91,57 +91,28 @@ public abstract class LuceneQueriesBase extends JUnit4CacheTestCase { dataStore2.invoke(() -> initDataStore(createIndex)); accessor.invoke(() -> initAccessor(createIndex)); - try { - dataStore1.invoke(() -> setTestHook()); - putDataInRegion(accessor); - waitForFlushBeforeExecuteTextSearch(accessor, 10); - executeTextSearch(accessor); - } finally { - dataStore1.invoke(() -> checkResultAndresetTestHook()); - } + //Pause the sender to make sure some entries are queued up + dataStore1.invoke(() -> pauseSender(getCache())); + dataStore2.invoke(() -> pauseSender(getCache())); + putDataInRegion(accessor); + assertFalse(waitForFlushBeforeExecuteTextSearch(accessor, 500)); + dataStore1.invoke(() -> resumeSender(getCache())); + dataStore2.invoke(() -> resumeSender(getCache())); + assertTrue(waitForFlushBeforeExecuteTextSearch(accessor, 60000)); + executeTextSearch(accessor); } - protected void waitForFlushBeforeExecuteTextSearch(VM vm, final int expectKeyNum) { - vm.invoke(() -> { + protected boolean waitForFlushBeforeExecuteTextSearch(VM vm, int ms) { + return vm.invoke(() -> { Cache cache = getCache(); - Region<Object, Object> region = cache.getRegion(REGION_NAME); LuceneService service = LuceneServiceProvider.get(cache); LuceneIndexImpl index = (LuceneIndexImpl)service.getIndex(INDEX_NAME, REGION_NAME); - assertNotNull(index); - LuceneQuery<Integer, TestObject> query; - String aeqId = LuceneServiceImpl.getUniqueIndexName(INDEX_NAME, REGION_NAME); - AsyncEventQueue queue = cache.getAsyncEventQueue(aeqId); - assertNotNull(queue); - assertTrue(queue.size()>0); - index.waitUntilFlushed(30000); - return null; + return index.waitUntilFlushed(ms); }); } - public static void setTestHook() { - LuceneEventListener.testHook = new LuceneEventListener.TestHook() { - - @Override - public void doTestHook(String spot) { - if (spot.equals("FOUND_AND_BEFORE_PROCESSING_A_EVENT")) { - try { - Thread.sleep(1000); - LogService.getLogger().debug("Waited in test hook"); - } - catch (InterruptedException e) { - } - } - } - }; - } - - public static void checkResultAndresetTestHook() - { - LuceneEventListener.testHook = null; - } - protected void executeTextSearch(VM vm) { vm.invoke(() -> { Cache cache = getCache(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java index fbd101e..4d5a0b7 100644 --- a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java +++ b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPRBase.java @@ -19,6 +19,7 @@ package com.gemstone.gemfire.cache.lucene; +import static com.gemstone.gemfire.cache.lucene.test.LuceneTestUtilities.*; import static org.junit.Assert.*; import java.io.Serializable; @@ -60,6 +61,7 @@ public abstract class LuceneQueriesPRBase extends LuceneQueriesBase { dataStore2.invoke(() -> initDataStore(createIndex)); rebalanceRegion(dataStore1); + assertTrue(waitForFlushBeforeExecuteTextSearch(accessor, 60000)); executeTextSearch(accessor); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPRDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPRDUnitTest.java b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPRDUnitTest.java index 51d0a33..830ca26 100644 --- a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPRDUnitTest.java +++ b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPRDUnitTest.java @@ -16,6 +16,7 @@ */ package com.gemstone.gemfire.cache.lucene; +import static com.gemstone.gemfire.cache.lucene.test.LuceneTestUtilities.REGION_NAME; import com.gemstone.gemfire.cache.RegionShortcut; import com.gemstone.gemfire.test.dunit.SerializableRunnableIF; import com.gemstone.gemfire.test.junit.categories.DistributedTest; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflowDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflowDUnitTest.java b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflowDUnitTest.java index cf2bac7..8fd0a08 100644 --- a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflowDUnitTest.java +++ b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesPeerPROverflowDUnitTest.java @@ -16,6 +16,8 @@ */ package com.gemstone.gemfire.cache.lucene; +import static com.gemstone.gemfire.cache.lucene.test.LuceneTestUtilities.*; + import com.gemstone.gemfire.cache.EvictionAction; import com.gemstone.gemfire.cache.EvictionAttributes; import com.gemstone.gemfire.cache.RegionShortcut; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/4a97daf6/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java index 7a3ef04..571049c 100644 --- a/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java +++ b/geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/test/LuceneTestUtilities.java @@ -28,6 +28,7 @@ import java.util.stream.Collectors; import com.gemstone.gemfire.cache.Cache; import com.gemstone.gemfire.cache.asyncqueue.AsyncEventQueue; +import com.gemstone.gemfire.cache.asyncqueue.internal.AsyncEventQueueImpl; import com.gemstone.gemfire.cache.lucene.LuceneIndex; import com.gemstone.gemfire.cache.lucene.LuceneQuery; import com.gemstone.gemfire.cache.lucene.LuceneQueryResults; @@ -82,4 +83,14 @@ public class LuceneTestUtilities { } assertEquals(expectedKeySet, actualKeySet); } + + public static void pauseSender(final Cache cache) { + final AsyncEventQueueImpl queue = (AsyncEventQueueImpl) getIndexQueue(cache); + queue.getSender().pause(); + } + + public static void resumeSender(final Cache cache) { + final AsyncEventQueueImpl queue = (AsyncEventQueueImpl) getIndexQueue(cache); + queue.getSender().resume(); + } }