This is an automated email from the ASF dual-hosted git repository.
ladyvader pushed a commit to branch feature/GEODE-4764
in repository https://gitbox.apache.org/repos/asf/geode.git
commit ffd4351baaf3a965590919d2013d3f99c25a10f0
Author: Lynn Hughes-Godfrey <lhughesgodf...@pivotal.io>
AuthorDate: Thu Mar 1 17:22:23 2018 -0800
GEODE-4764: Address NPEs during Lucene index creation on existing region
* Don't add the aeq to the region until the data region has been created
* When iterating over list of local primary bucketIds (during reindexing),
check for null return value when retrieving a specific bucket
* Added tests for same.
---
.../cache/lucene/internal/LuceneServiceImpl.java | 7 +-
.../internal/LuceneServiceImplJUnitTest.java | 77 +++++++++++++++++++++-
2 files changed, 80 insertions(+), 4 deletions(-)
diff --git
a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
index 2ac0df4..44d2e11 100644
---
a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
+++
b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
@@ -264,7 +264,6 @@ public class LuceneServiceImpl implements
InternalLuceneService {
validateRegionAttributes(region.getAttributes());
String aeqId = LuceneServiceImpl.getUniqueIndexName(indexName, regionPath);
- region.addAsyncEventQueueId(aeqId, true);
region.addCacheServiceProfile(new LuceneIndexCreationProfile(indexName,
regionPath, fields,
analyzer, fieldAnalyzers, serializer));
@@ -274,6 +273,8 @@ public class LuceneServiceImpl implements
InternalLuceneService {
afterDataRegionCreated(luceneIndex);
+ region.addAsyncEventQueueId(aeqId, true);
+
createLuceneIndexOnDataRegion(region, luceneIndex);
}
@@ -291,6 +292,10 @@ public class LuceneServiceImpl implements
InternalLuceneService {
int primaryBucketId = (Integer) primaryBucketIterator.next();
try {
BucketRegion userBucket =
userRegion.getDataStore().getLocalBucketById(primaryBucketId);
+ if (userBucket == null) {
+ throw new BucketNotFoundException(
+ "Bucket ID : " + primaryBucketId + " not found during lucene
indexing");
+ }
if (!userBucket.isEmpty()) {
/**
*
diff --git
a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneServiceImplJUnitTest.java
b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneServiceImplJUnitTest.java
index 23c23cc..1b799f5 100644
---
a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneServiceImplJUnitTest.java
+++
b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneServiceImplJUnitTest.java
@@ -16,13 +16,24 @@ package org.apache.geode.cache.lucene.internal;
import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import java.lang.reflect.Field;
+import java.util.Arrays;
import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
import java.util.concurrent.TimeUnit;
+import org.apache.lucene.analysis.Analyzer;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -30,10 +41,19 @@ import org.junit.experimental.categories.Category;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;
-import org.apache.geode.cache.Region;
+import org.apache.geode.Statistics;
+import org.apache.geode.StatisticsFactory;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EvictionAlgorithm;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.RegionAttributes;
+import org.apache.geode.cache.asyncqueue.internal.AsyncEventQueueFactoryImpl;
import org.apache.geode.cache.lucene.LuceneIndexFactory;
import org.apache.geode.cache.lucene.LuceneSerializer;
+import org.apache.geode.distributed.DistributedSystem;
import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.cache.PartitionedRegion;
+import org.apache.geode.internal.cache.PartitionedRegionDataStore;
import org.apache.geode.test.junit.categories.UnitTest;
@Category(UnitTest.class)
@@ -41,14 +61,14 @@ public class LuceneServiceImplJUnitTest {
@Rule
public ExpectedException thrown = ExpectedException.none();
- Region region;
+ PartitionedRegion region;
GemFireCacheImpl cache;
LuceneServiceImpl service = new LuceneServiceImpl();
@Before
public void createMocks() throws NoSuchFieldException, SecurityException,
IllegalArgumentException, IllegalAccessException {
- region = mock(Region.class);
+ region = mock(PartitionedRegion.class);
cache = mock(GemFireCacheImpl.class);
Field f = LuceneServiceImpl.class.getDeclaredField("cache");
f.setAccessible(true);
@@ -86,4 +106,55 @@ public class LuceneServiceImplJUnitTest {
assertFalse(result);
}
+ @Test
+ public void userRegionShouldNotBeSetBeforeIndexInitialized() throws
Exception {
+ TestLuceneServiceImpl testService = new TestLuceneServiceImpl();
+ Field f = LuceneServiceImpl.class.getDeclaredField("cache");
+ f.setAccessible(true);
+ f.set(testService, cache);
+ AsyncEventQueueFactoryImpl aeqFactory =
mock(AsyncEventQueueFactoryImpl.class);
+ when(cache.createAsyncEventQueueFactory()).thenReturn(aeqFactory);
+
+ DistributedSystem ds = mock(DistributedSystem.class);
+ Statistics luceneIndexStats = mock(Statistics.class);
+ when(cache.getDistributedSystem()).thenReturn(ds);
+ when(((StatisticsFactory) ds).createAtomicStatistics(any(), anyString()))
+ .thenReturn(luceneIndexStats);
+ when(cache.getRegion(anyString())).thenReturn(region);
+
+ RegionAttributes ratts = mock(RegionAttributes.class);
+ when(region.getAttributes()).thenReturn(ratts);
+ when(ratts.getDataPolicy()).thenReturn(DataPolicy.PARTITION);
+ EvictionAttributes evictionAttrs = mock(EvictionAttributes.class);
+ when(ratts.getEvictionAttributes()).thenReturn(evictionAttrs);
+ when(evictionAttrs.getAlgorithm()).thenReturn(EvictionAlgorithm.NONE);
+
+ Map<String, Analyzer> fieldMap = new HashMap<String, Analyzer>();
+ fieldMap.put("field1", null);
+ fieldMap.put("field2", null);
+ testService.createIndex("index", "region", fieldMap, null, true);
+ }
+
+ @Test
+ public void
createLuceneIndexOnExistingRegionShouldNotThrowNPEIfBucketMovedDuringReindexing()
{
+ LuceneIndexImpl index = mock(LuceneIndexImpl.class);
+ PartitionedRegionDataStore dataStore =
mock(PartitionedRegionDataStore.class);
+ when(region.getDataStore()).thenReturn(dataStore);
+ Integer bucketIds[] = {1, 2, 3, 4, 5};
+ Set<Integer> primaryBucketIds = new HashSet(Arrays.asList(bucketIds));
+ when(dataStore.getAllLocalPrimaryBucketIds()).thenReturn(primaryBucketIds);
+ when(dataStore.getLocalBucketById(3)).thenReturn(null);
+ boolean result = service.createLuceneIndexOnDataRegion(region, index);
+ assertTrue(result);
+ }
+
+ private class TestLuceneServiceImpl extends LuceneServiceImpl {
+
+ @Override
+ public void afterDataRegionCreated(LuceneIndexImpl index) {
+ PartitionedRegion userRegion =
+ (PartitionedRegion)
index.getCache().getRegion(index.getRegionPath());
+ verify(userRegion, never()).addAsyncEventQueueId(anyString(),
anyBoolean());
+ }
+ }
}
--
To stop receiving notification emails like this one, please contact
ladyva...@apache.org.
