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.