You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2016/12/14 17:57:19 UTC
[04/50] [abbrv] geode git commit: GEODE-2182: try-catch around size
estimation for index look ups
GEODE-2182: try-catch around size estimation for index look ups
* Fix possible race condition where entry is destroyed
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/c2b1c8b1
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/c2b1c8b1
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/c2b1c8b1
Branch: refs/heads/feature/GEODE-1930
Commit: c2b1c8b1c172073824a8e5c48c45a4285f15e6b1
Parents: ef96dba
Author: Jason Huynh <hu...@gmail.com>
Authored: Mon Dec 5 13:53:22 2016 -0800
Committer: Jason Huynh <hu...@gmail.com>
Committed: Wed Dec 7 09:28:38 2016 -0800
----------------------------------------------------------------------
.../query/internal/index/CompactRangeIndex.java | 2 +
.../query/internal/index/MemoryIndexStore.java | 12 ++-
.../functional/IndexOnEntrySetJUnitTest.java | 89 ++++++++++++++------
3 files changed, 68 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/c2b1c8b1/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
index a8a38bd..80568f5 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
@@ -437,6 +437,8 @@ public class CompactRangeIndex extends AbstractIndex {
}
break;
}
+ } catch (EntryDestroyedException e) {
+ return Integer.MAX_VALUE;
} finally {
updateIndexUseEndStats(start, false);
}
http://git-wip-us.apache.org/repos/asf/geode/blob/c2b1c8b1/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
index 3635897..e9cd070 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
@@ -247,10 +247,7 @@ public class MemoryIndexStore implements IndexStore {
* Find the old key by traversing the forward map in case of in-place update modification If not
* found it means the value object was modified with same value. So oldKey is same as newKey.
*
- * @param newKey
- * @param entry
* @return oldKey
- * @throws TypeMismatchException
*/
private Object getOldKey(Object newKey, RegionEntry entry) throws TypeMismatchException {
for (Object mapEntry : valueToEntriesMap.entrySet()) {
@@ -406,7 +403,6 @@ public class MemoryIndexStore implements IndexStore {
* RegionEntry) { return Collections.singleton(regionEntries); } return (Collection)
* regionEntries; }
*/
-
@Override
public CloseableIterator<IndexStoreEntry> get(Object indexKey) {
return new MemoryIndexStoreIterator(
@@ -574,7 +570,6 @@ public class MemoryIndexStore implements IndexStore {
/**
* A bi-directional iterator over the CSL. Iterates over the entries of CSL where entry is a
* mapping (value -> Collection) as well as over the Collection.
- *
*/
private class MemoryIndexStoreIterator implements CloseableIterator<IndexStoreEntry> {
final Map map;
@@ -662,8 +657,9 @@ public class MemoryIndexStore implements IndexStore {
}
RegionEntry re = (RegionEntry) valuesIterator.next();
- if (re == null)
+ if (re == null) {
throw new NoSuchElementException();
+ }
currentEntry.setMemoryIndexStoreEntry(currKey, re);
return currentEntry;
@@ -718,7 +714,6 @@ public class MemoryIndexStore implements IndexStore {
/**
* A wrapper over the entry in the CSL index map. It maps IndexKey -> RegionEntry
- *
*/
class MemoryIndexStoreEntry implements IndexStoreEntry {
private Object deserializedIndexKey;
@@ -771,6 +766,9 @@ public class MemoryIndexStore implements IndexStore {
private Object key, value;
public CachedEntryWrapper(LocalRegion.NonTXEntry entry) {
+ if (IndexManager.testHook != null) {
+ IndexManager.testHook.hook(201);
+ }
this.key = entry.getKey();
this.value = entry.getValue();
}
http://git-wip-us.apache.org/repos/asf/geode/blob/c2b1c8b1/geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexOnEntrySetJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexOnEntrySetJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexOnEntrySetJUnitTest.java
index 6034d53..f659836 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexOnEntrySetJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexOnEntrySetJUnitTest.java
@@ -14,6 +14,10 @@
*/
package org.apache.geode.cache.query.functional;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
import org.apache.geode.cache.*;
import org.apache.geode.cache.query.*;
import org.apache.geode.cache.query.internal.QueryObserverAdapter;
@@ -91,7 +95,15 @@ public class IndexOnEntrySetJUnitTest {
public void testQueriesOnReplicatedRegion() throws Exception {
testRegion = createReplicatedRegion(testRegionName);
String regionPath = "/" + testRegionName + ".entrySet entry";
- executeQueryTest(getQueriesOnRegion(testRegionName), "entry.key.Index", regionPath);
+ executeQueryTest(getQueriesOnRegion(testRegionName), "entry.key.Index", regionPath, 200);
+ }
+
+ @Test
+ public void testEntryDestroyedRaceWithSizeEstimateReplicatedRegion() throws Exception {
+ testRegion = createReplicatedRegion(testRegionName);
+ String regionPath = "/" + testRegionName + ".entrySet entry";
+ executeQueryTestDestroyDuringSizeEstimation(getQueriesOnRegion(testRegionName),
+ "entry.key.Index", regionPath, 201);
}
/**
@@ -102,7 +114,7 @@ public class IndexOnEntrySetJUnitTest {
public void testQueriesOnPartitionedRegion() throws Exception {
testRegion = createPartitionedRegion(testRegionName);
String regionPath = "/" + testRegionName + ".entrySet entry";
- executeQueryTest(getQueriesOnRegion(testRegionName), "entry.key.Index", regionPath);
+ executeQueryTest(getQueriesOnRegion(testRegionName), "entry.key.Index", regionPath, 200);
}
private Region createReplicatedRegion(String regionName) throws ParseException {
@@ -140,24 +152,26 @@ public class IndexOnEntrySetJUnitTest {
}
}
- /**** Query Execution Helpers ****/
+ /****
+ * Query Execution Helpers
+ ****/
- private void executeQueryTest(String[] queries, String indexedExpression, String regionPath)
- throws Exception {
+ private void executeQueryTest(String[] queries, String indexedExpression, String regionPath,
+ int testHookSpot) throws Exception {
Cache cache = CacheUtils.getCache();
boolean[] booleanVals = {true, false};
for (String query : queries) {
for (boolean isDestroy : booleanVals) {
clearData(testRegion);
populateRegion(testRegion);
- Assert.assertNotNull(cache.getRegion(testRegionName));
- Assert.assertEquals(numElem, cache.getRegion(testRegionName).size());
+ assertNotNull(cache.getRegion(testRegionName));
+ assertEquals(numElem, cache.getRegion(testRegionName).size());
if (isDestroy) {
helpTestFunctionalIndexForQuery(query, indexedExpression, regionPath,
- new DestroyEntryTestHook(testRegion));
+ new DestroyEntryTestHook(testRegion, testHookSpot), 1);
} else {
helpTestFunctionalIndexForQuery(query, indexedExpression, regionPath,
- new InvalidateEntryTestHook(testRegion));
+ new InvalidateEntryTestHook(testRegion, testHookSpot), 1);
}
}
}
@@ -166,22 +180,18 @@ public class IndexOnEntrySetJUnitTest {
for (String query : queries) {
clearData(testRegion);
populateRegion(testRegion);
- Assert.assertNotNull(cache.getRegion(testRegionName));
- Assert.assertEquals(numElem, cache.getRegion(testRegionName).size());
+ assertNotNull(cache.getRegion(testRegionName));
+ assertEquals(numElem, cache.getRegion(testRegionName).size());
helpTestFunctionalIndexForQuery(query, indexedExpression, regionPath,
- new PutEntryTestHook(testRegion));
+ new PutEntryTestHook(testRegion, testHookSpot), 1);
}
}
/**
* helper method to test against a functional index make sure there is no UNDEFINED result
- *
- * @param query
- *
- * @throws Exception
*/
private SelectResults helpTestFunctionalIndexForQuery(String query, String indexedExpression,
- String regionPath, AbstractTestHook testHook) throws Exception {
+ String regionPath, AbstractTestHook testHook, int expectedSize) throws Exception {
MyQueryObserverAdapter observer = new MyQueryObserverAdapter();
QueryObserverHolder.setInstance(observer);
IndexManager.testHook = testHook;
@@ -194,27 +204,40 @@ public class IndexOnEntrySetJUnitTest {
if (row instanceof Struct) {
Object[] fields = ((Struct) row).getFieldValues();
for (Object field : fields) {
- Assert.assertTrue(field != QueryService.UNDEFINED);
+ assertTrue(field != QueryService.UNDEFINED);
if (field instanceof String) {
- Assert.assertTrue(((String) field).compareTo(newValue) != 0);
+ assertTrue(((String) field).compareTo(newValue) != 0);
}
}
} else {
- Assert.assertTrue(row != QueryService.UNDEFINED);
+ assertTrue(row != QueryService.UNDEFINED);
if (row instanceof String) {
- Assert.assertTrue(((String) row).compareTo(newValue) != 0);
+ assertTrue(((String) row).compareTo(newValue) != 0);
}
}
}
- Assert.assertTrue(indexedResults.size() > 0);
- Assert.assertTrue(observer.indexUsed);
- Assert.assertTrue(((AbstractTestHook) IndexManager.testHook).isTestHookCalled());
+ assertTrue(indexedResults.size() >= expectedSize);
+ assertTrue(observer.indexUsed);
+ assertTrue(((AbstractTestHook) IndexManager.testHook).isTestHookCalled());
((AbstractTestHook) IndexManager.testHook).reset();
qs.removeIndex(index);
return indexedResults;
}
+ private void executeQueryTestDestroyDuringSizeEstimation(String[] queries,
+ String indexedExpression, String regionPath, int testHookSpot) throws Exception {
+ Cache cache = CacheUtils.getCache();
+ for (String query : queries) {
+ clearData(testRegion);
+ populateRegion(testRegion);
+ assertNotNull(cache.getRegion(testRegionName));
+ assertEquals(numElem, cache.getRegion(testRegionName).size());
+ helpTestFunctionalIndexForQuery(query, indexedExpression, regionPath,
+ new DestroyEntryTestHook(testRegion, testHookSpot), 0);
+ }
+ }
+
class MyQueryObserverAdapter extends QueryObserverAdapter {
public boolean indexUsed = false;
@@ -255,6 +278,12 @@ public class IndexOnEntrySetJUnitTest {
Object waitObj = new Object();
Region r;
+ private int testHookSpot;
+
+ public AbstractTestHook(int testHookSpot) {
+ this.testHookSpot = testHookSpot;
+ }
+
public void reset() {
isTestHookCalled = false;
}
@@ -270,7 +299,7 @@ public class IndexOnEntrySetJUnitTest {
@Override
public void hook(int spot) throws RuntimeException {
- if (spot == 200) {
+ if (spot == testHookSpot) {
if (!isTestHookCalled) {
isTestHookCalled = true;
try {
@@ -297,7 +326,9 @@ public class IndexOnEntrySetJUnitTest {
class DestroyEntryTestHook extends AbstractTestHook {
- DestroyEntryTestHook(Region r) {
+ DestroyEntryTestHook(Region r, int testHookSpot) {
+
+ super(testHookSpot);
this.r = r;
}
@@ -312,7 +343,8 @@ public class IndexOnEntrySetJUnitTest {
class InvalidateEntryTestHook extends AbstractTestHook {
- InvalidateEntryTestHook(Region r) {
+ InvalidateEntryTestHook(Region r, int testHookSpot) {
+ super(testHookSpot);
this.r = r;
}
@@ -327,7 +359,8 @@ public class IndexOnEntrySetJUnitTest {
class PutEntryTestHook extends AbstractTestHook {
- PutEntryTestHook(Region r) {
+ PutEntryTestHook(Region r, int testHookSpot) {
+ super(testHookSpot);
this.r = r;
}