GEODE-3115 Added changes to check for persistent region during pdx type registry.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/06753177 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/06753177 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/06753177 Branch: refs/heads/feature/GEM-1483 Commit: 067531776f73828d6dfa981096641539e56e6c0a Parents: e7515f5 Author: Anil <[email protected]> Authored: Tue Jul 25 10:08:15 2017 -0700 Committer: Anil <[email protected]> Committed: Tue Jul 25 10:09:40 2017 -0700 ---------------------------------------------------------------------- .../geode/internal/cache/GemFireCacheImpl.java | 23 +- .../geode/internal/cache/InternalCache.java | 2 + .../internal/cache/xmlcache/CacheCreation.java | 5 + .../pdx/internal/PeerTypeRegistration.java | 18 +- .../geode/pdx/PdxAttributesJUnitTest.java | 239 ++++++++++++------- 5 files changed, 184 insertions(+), 103 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/06753177/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java index de5fd88..f176d22 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java @@ -76,13 +76,12 @@ import javax.transaction.TransactionManager; import com.sun.jna.Native; import com.sun.jna.Platform; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.StringUtils; +import org.apache.logging.log4j.Logger; import org.apache.geode.internal.cache.event.EventTracker; import org.apache.geode.internal.cache.event.EventTrackerExpiryTask; import org.apache.geode.internal.security.SecurityServiceFactory; -import org.apache.logging.log4j.Logger; - import org.apache.geode.CancelCriterion; import org.apache.geode.CancelException; import org.apache.geode.ForcedDisconnectException; @@ -3216,6 +3215,24 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has return result; } + @SuppressWarnings("unchecked") + @Override + public boolean hasPersistentRegion() { + synchronized (this.rootRegions) { + for (LocalRegion region : this.rootRegions.values()) { + if (region.getDataPolicy().withPersistence()) { + return true; + } + for (LocalRegion subRegion : (Set<LocalRegion>) region.basicSubregions(true)) { + if (subRegion.getDataPolicy().withPersistence()) { + return true; + } + } + } + return false; + } + } + @Override public void setRegionByPath(String path, LocalRegion r) { if (r == null) { http://git-wip-us.apache.org/repos/asf/geode/blob/06753177/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java index aed439c..d162010 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java @@ -313,4 +313,6 @@ public interface InternalCache extends Cache, Extensible<Cache>, CacheTime { void waitForRegisterInterestsInProgress(); SecurityService getSecurityService(); + + boolean hasPersistentRegion(); } http://git-wip-us.apache.org/repos/asf/geode/blob/06753177/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java index db5f7ca..a7f2a11 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java @@ -2204,4 +2204,9 @@ public class CacheCreation implements InternalCache { public URL getCacheXmlURL() { throw new UnsupportedOperationException(LocalizedStrings.SHOULDNT_INVOKE.toLocalizedString()); } + + @Override + public boolean hasPersistentRegion() { + throw new UnsupportedOperationException(LocalizedStrings.SHOULDNT_INVOKE.toLocalizedString()); + } } http://git-wip-us.apache.org/repos/asf/geode/blob/06753177/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java index 065255b..ea9bf3e 100644 --- a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java +++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java @@ -480,7 +480,7 @@ public class PeerTypeRegistration implements TypeRegistration { if (!typeRegistryInUse || this.idToType == null) { return; } - checkAllowed(true, hasPersistentRegions()); + checkAllowed(true, this.cache.hasPersistentRegion()); } public void creatingPersistentRegion() { @@ -514,8 +514,7 @@ public class PeerTypeRegistration implements TypeRegistration { if (typeRegistryInUse) { return; } else { - boolean hasPersistentRegions = hasPersistentRegions(); - checkAllowed(hasGatewaySender(), hasPersistentRegions); + checkAllowed(hasGatewaySender(), this.cache.hasPersistentRegion()); for (Pool pool : PoolManager.getAll().values()) { if (!((PoolImpl) pool).isUsedByGateway()) { @@ -529,17 +528,8 @@ public class PeerTypeRegistration implements TypeRegistration { } } - public boolean hasPersistentRegions() { - Collection<DiskStore> diskStores = cache.listDiskStoresIncludingRegionOwned(); - boolean hasPersistentRegions = false; - for (DiskStore store : diskStores) { - hasPersistentRegions |= ((DiskStoreImpl) store).hasPersistedData(); - } - return hasPersistentRegions; - } - - private void checkAllowed(boolean hasGatewaySender, boolean hasDiskStore) { - if (hasDiskStore && !cache.getPdxPersistent()) { + private void checkAllowed(boolean hasGatewaySender, boolean hasPersistentRegion) { + if (hasPersistentRegion && !cache.getPdxPersistent()) { throw new PdxInitializationException( "The PDX metadata must be persistent in a member that has persistent data. See CacheFactory.setPdxPersistent."); } http://git-wip-us.apache.org/repos/asf/geode/blob/06753177/geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java b/geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java index ef15cd5..114039d 100644 --- a/geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java @@ -14,7 +14,10 @@ */ package org.apache.geode.pdx; +import static com.googlecode.catchexception.CatchException.catchException; +import static com.googlecode.catchexception.CatchException.caughtException; import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import org.apache.commons.io.FileUtils; @@ -26,12 +29,15 @@ import org.apache.geode.cache.CacheWriterException; import org.apache.geode.cache.DataPolicy; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.cache.asyncqueue.AsyncEvent; +import org.apache.geode.cache.asyncqueue.AsyncEventListener; import org.apache.geode.cache.client.PoolManager; import org.apache.geode.distributed.ConfigurationProperties; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.internal.HeapDataOutputStream; import org.apache.geode.internal.Version; import org.apache.geode.internal.cache.GemFireCacheImpl; +import org.apache.geode.pdx.PdxInitializationException; import org.apache.geode.pdx.SimpleClass.SimpleEnum; import org.apache.geode.pdx.internal.EnumId; import org.apache.geode.pdx.internal.EnumInfo; @@ -48,6 +54,7 @@ import java.io.File; import java.io.FilenameFilter; import java.io.IOException; import java.util.Iterator; +import java.util.List; import java.util.Map; /** @@ -58,6 +65,8 @@ public class PdxAttributesJUnitTest { private File diskDir; + private Cache cache; + @Before public void setUp() { diskDir = new File("PdxAttributesJUnitTest"); @@ -82,15 +91,15 @@ public class PdxAttributesJUnitTest { for (File file : defaultStoreFiles) { FileUtils.forceDelete(file); } + if (cache != null) { + cache.close(); + } } @Test public void testPdxPersistent() throws Exception { { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); - + initCache(false, false, null); // define a type defineAType(); Region pdxRegion = cache.getRegion(PeerTypeRegistration.REGION_NAME); @@ -101,28 +110,18 @@ public class PdxAttributesJUnitTest { setUp(); { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - cf.setPdxPersistent(true); - Cache cache = cf.create(); - + initCache(false, true, null); // define a type defineAType(); Region pdxRegion = cache.getRegion(PeerTypeRegistration.REGION_NAME); assertEquals(DataPolicy.PERSISTENT_REPLICATE, pdxRegion.getAttributes().getDataPolicy()); - cache.close(); } } @Test public void testPdxTypeId() throws Exception { - int dsId = 5; - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - cf.set(ConfigurationProperties.DISTRIBUTED_SYSTEM_ID, String.valueOf(dsId)); - Cache cache = cf.create(); - + initCache(false, false, String.valueOf(dsId)); // define a type. defineAType(); @@ -166,13 +165,8 @@ public class PdxAttributesJUnitTest { @Test public void testDuplicatePdxTypeId() throws Exception { - int dsId = 5; - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - cf.set(ConfigurationProperties.DISTRIBUTED_SYSTEM_ID, String.valueOf(dsId)); - Cache cache = cf.create(); - + initCache(false, false, String.valueOf(dsId)); // define a type. defineAType(); @@ -213,11 +207,7 @@ public class PdxAttributesJUnitTest { public void testPdxTypeIdWithNegativeDsId() throws Exception { // in this case geode will use 0 as dsId int dsId = -1; - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - cf.set(ConfigurationProperties.DISTRIBUTED_SYSTEM_ID, String.valueOf(dsId)); - Cache cache = cf.create(); - + initCache(false, false, String.valueOf(dsId)); // define a type. defineAType(); @@ -260,13 +250,7 @@ public class PdxAttributesJUnitTest { @Test public void testPdxDiskStore() throws Exception { { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - cf.setPdxPersistent(true); - cf.setPdxDiskStore("diskstore1"); - Cache cache = cf.create(); - cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) - .create("diskstore1"); + initCache(true, true, null, "diskstore1"); // define a type. defineAType(); @@ -279,11 +263,7 @@ public class PdxAttributesJUnitTest { setUp(); { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - cf.setPdxPersistent(true); - Cache cache = cf.create(); - + initCache(false, true, null); // define a type defineAType(); Region pdxRegion = cache.getRegion(PeerTypeRegistration.REGION_NAME); @@ -295,11 +275,7 @@ public class PdxAttributesJUnitTest { @Test public void testNonPersistentRegistryWithOverflowRegion() throws Exception { { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); - cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) - .create("diskstore1"); + initCache(true, false, null, "diskstore1"); cache.createRegionFactory(RegionShortcut.LOCAL_OVERFLOW).setDiskStoreName("diskstore1") .create("region"); defineAType(); @@ -308,9 +284,7 @@ public class PdxAttributesJUnitTest { setUp(); { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); + initCache(false, false, null); defineAType(); cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) .create("diskstore1"); @@ -322,39 +296,90 @@ public class PdxAttributesJUnitTest { @Test public void testNonPersistentRegistryWithPersistentRegion() throws Exception { { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); - cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) - .create("diskstore1"); - cache.createRegionFactory(RegionShortcut.LOCAL_PERSISTENT).setDiskStoreName("diskstore1") - .create("region"); + initCache(true, false, null); + Region region = createRegion(true, false); - try { - defineATypeNoEnum(); - throw new RuntimeException("Should have received an exception"); - } catch (PdxInitializationException expected) { + catchException(this).defineATypeNoEnum(); + assertThat((Exception) caughtException()) + .isExactlyInstanceOf(PdxInitializationException.class); - } + // Drop partitioned region. + region.destroyRegion(); + // The pdx type creation should work. + defineATypeNoEnum(); } tearDown(); setUp(); + { + initCache(true, false, null); + defineATypeNoEnum(); + catchException(this).createRegion(true, false); + assertThat((Exception) caughtException()) + .isExactlyInstanceOf(PdxInitializationException.class); + } + } + @Test + public void testNonPersistentRegistryWithPersistentPR() throws Exception { { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); + initCache(true, false, null); + Region region = createRegion(true, true); + catchException(this).defineATypeNoEnum(); + assertThat((Exception) caughtException()) + .isExactlyInstanceOf(PdxInitializationException.class); + + // Drop partitioned region. + region.destroyRegion(); + // The pdx type creation should work. defineATypeNoEnum(); - cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) - .create("diskstore1"); - try { - cache.createRegionFactory(RegionShortcut.LOCAL_PERSISTENT).setDiskStoreName("diskStore1") - .create("region"); - throw new RuntimeException("Should have received an exception"); - } catch (PdxInitializationException expected) { + } + tearDown(); + setUp(); - } + { + initCache(true, false, null); + defineATypeNoEnum(); + catchException(this).createRegion(true, true); + assertThat((Exception) caughtException()) + .isExactlyInstanceOf(PdxInitializationException.class); + } + } + + @Test + public void testPersistentRegistryWithPersistentRegion() throws Exception { + { + initCache(true, true, null); + createRegion(true, false); + defineATypeNoEnum(); + } + tearDown(); + setUp(); + + { + initCache(true, true, null); + defineATypeNoEnum(); + createRegion(true, false); + } + } + @Test + public void testNonPersistentRegistryWithAEQ() throws Exception { + { + initCache(true, false, null); + definePersistentAEQ(cache, "aeq", true); + catchException(this).defineATypeNoEnum(); + assertThat((Exception) caughtException()) + .isExactlyInstanceOf(PdxInitializationException.class); + } + tearDown(); + setUp(); + + { + initCache(true, false, null); + defineATypeNoEnum(); + catchException(this).definePersistentAEQ(cache, "aeq", true); + assertThat((Exception) caughtException()) + .isExactlyInstanceOf(PdxInitializationException.class); } } @@ -367,9 +392,7 @@ public class PdxAttributesJUnitTest { public void testLazyLoner() throws Exception { // Test that we can become a peer registry { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); + initCache(false, false, null); // This should work, because this is a peer. defineAType(); } @@ -378,33 +401,77 @@ public class PdxAttributesJUnitTest { // Test that we can become a client registry. { - CacheFactory cf = new CacheFactory(); - cf.set(MCAST_PORT, "0"); - Cache cache = cf.create(); + initCache(false, false, null); int port = AvailablePortHelper.getRandomAvailableTCPPort(); PoolManager.createFactory().addServer("localhost", port).create("pool"); - - try { - defineAType(); - throw new RuntimeException( - "Should have failed, this is a client that can't connect to a server"); - } catch (ToDataException expected) { - // do nothing. - } + catchException(this).defineAType(); + assertThat((Exception) caughtException()).isExactlyInstanceOf(ToDataException.class); } } - private void defineAType() throws IOException { + public void defineAType() throws IOException { SimpleClass sc = new SimpleClass(1, (byte) 2); HeapDataOutputStream out = new HeapDataOutputStream(Version.CURRENT); DataSerializer.writeObject(sc, out); } - private void defineATypeNoEnum() throws IOException { + public void defineATypeNoEnum() throws /* IO */ Exception { SimpleClass sc = new SimpleClass(1, (byte) 2, null); HeapDataOutputStream out = new HeapDataOutputStream(Version.CURRENT); DataSerializer.writeObject(sc, out); } + private void initCache(boolean createDiskStore, boolean pdxPersist, String dsId) { + initCache(createDiskStore, pdxPersist, dsId, null); + } + + private void initCache(boolean createDiskStore, boolean pdxPersist, String dsId, + String pdxDiskstore) { + CacheFactory cf = new CacheFactory(); + cf.set(MCAST_PORT, "0"); + cf.setPdxPersistent(pdxPersist); + if (dsId != null) { + cf.set(ConfigurationProperties.DISTRIBUTED_SYSTEM_ID, dsId); + } + if (pdxDiskstore != null) { + cf.setPdxDiskStore(pdxDiskstore); + } + cache = cf.create(); + + if (createDiskStore) { + cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) + .create("diskstore1"); + } + } + + public Region createRegion(boolean persistent, boolean pr) { + Region region; + RegionShortcut rs; + + if (persistent) { + // cache.createDiskStoreFactory().setDiskDirs(new File[] {diskDir}).setMaxOplogSize(1) + // .create("diskstore1"); + + rs = pr ? RegionShortcut.PARTITION_PERSISTENT : RegionShortcut.LOCAL_PERSISTENT; + region = cache.createRegionFactory(rs).setDiskStoreName("diskstore1").create("region"); + } else { + rs = pr ? RegionShortcut.PARTITION : RegionShortcut.LOCAL; + region = cache.createRegionFactory(rs).create("region"); + } + return region; + } + + public void definePersistentAEQ(Cache cache, String id, boolean persistent) { + AsyncEventListener al = new AsyncEventListener() { + public void close() {} + + public boolean processEvents(List<AsyncEvent> events) { + return true; + } + }; + + cache.createAsyncEventQueueFactory().setPersistent(persistent).create(id, al); + } + }
