GEODE-2801: change krfIds to be thread safe
Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/003de33f Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/003de33f Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/003de33f Branch: refs/heads/feature/GEM-1299 Commit: 003de33f217f854a9090386ae704d50bac57fe09 Parents: c1a6914 Author: Darrel Schneider <[email protected]> Authored: Wed Apr 19 17:09:05 2017 -0700 Committer: zhouxh <[email protected]> Committed: Wed Apr 26 23:28:50 2017 -0700 ---------------------------------------------------------------------- .../geode/internal/cache/DiskInitFile.java | 10 ++-- .../org/apache/geode/internal/cache/Oplog.java | 1 + .../internal/cache/DiskInitFileJUnitTest.java | 57 ++++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/003de33f/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java index f6bf17f..0925d28 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java @@ -48,6 +48,7 @@ import org.apache.geode.internal.cache.persistence.PersistentMemberPattern; import org.apache.geode.internal.cache.versions.DiskRegionVersionVector; import org.apache.geode.internal.cache.versions.RegionVersionHolder; import org.apache.geode.internal.cache.versions.RegionVersionVector; +import org.apache.geode.internal.concurrent.ConcurrentHashSet; import org.apache.geode.internal.i18n.LocalizedStrings; import org.apache.geode.internal.logging.LogService; import org.apache.geode.internal.logging.log4j.LogMarker; @@ -72,6 +73,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -331,7 +333,9 @@ public class DiskInitFile implements DiskInitFileInterpreter { private final LongOpenHashSet crfIds; private final LongOpenHashSet drfIds; - private final LongOpenHashSet krfIds; + // krfIds uses a concurrent impl because backup + // can call hasKrf concurrently with cmnKrfCreate + private final ConcurrentHashSet<Long> krfIds; /** * Map used to keep track of regions we know of from the DiskInitFile but that do not yet exist @@ -1611,7 +1615,7 @@ public class DiskInitFile implements DiskInitFileInterpreter { } private void saveKrfIds() { - for (LongIterator i = this.krfIds.iterator(); i.hasNext();) { + for (Iterator<Long> i = this.krfIds.iterator(); i.hasNext();) { writeIFRecord(IFREC_KRF_CREATE, i.next()); this.ifLiveRecordCount++; this.ifTotalRecordCount++; @@ -1879,7 +1883,7 @@ public class DiskInitFile implements DiskInitFileInterpreter { this.instIds = new IntOpenHashSet(); this.crfIds = new LongOpenHashSet(); this.drfIds = new LongOpenHashSet(); - this.krfIds = new LongOpenHashSet(); + this.krfIds = new ConcurrentHashSet<>(); recover(); if (this.parent.isOffline() && !this.parent.isOfflineCompacting() && !this.parent.isOfflineModify()) { http://git-wip-us.apache.org/repos/asf/geode/blob/003de33f/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java index ca9468d..7f84393 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java @@ -5814,6 +5814,7 @@ public final class Oplog implements CompactableOplog, Flushable { } // this krf existence check fixes 45089 + // TODO: should we wait for the async KRF creation to finish by calling this.finishKrf? if (getParent().getDiskInitFile().hasKrf(this.oplogId)) { if (this.getKrfFile().exists()) { FileUtils.copyFileToDirectory(this.getKrfFile(), targetDir); http://git-wip-us.apache.org/repos/asf/geode/blob/003de33f/geode-core/src/test/java/org/apache/geode/internal/cache/DiskInitFileJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/DiskInitFileJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/DiskInitFileJUnitTest.java index 6f2cf6c..ec1b5cf 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/DiskInitFileJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/DiskInitFileJUnitTest.java @@ -117,4 +117,61 @@ public class DiskInitFileJUnitTest { dif.close(); } + @Test + public void testKrfIds() { + // create a mock statistics factory for creating directory holders + final StatisticsFactory sf = context.mock(StatisticsFactory.class); + context.checking(new Expectations() { + { + ignoring(sf); + } + }); + // Add a mock region to the init file so it doesn't + // delete the file when the init file is closed + final DiskRegionView drv = context.mock(DiskRegionView.class); + context.checking(new Expectations() { + { + ignoring(drv); + } + }); + // Create a mock disk store impl. All we need to do is return + // this init file directory. + final DiskStoreImpl parent = context.mock(DiskStoreImpl.class); + context.checking(new Expectations() { + { + allowing(parent).getInfoFileDir(); + will(returnValue(new DirectoryHolder(sf, testDirectory, 0, 0))); + ignoring(parent); + } + }); + + DiskInitFile dif = new DiskInitFile("testKrfIds", parent, false, Collections.<File>emptySet()); + assertEquals(false, dif.hasKrf(1)); + dif.cmnKrfCreate(1); + assertEquals(true, dif.hasKrf(1)); + assertEquals(false, dif.hasKrf(2)); + dif.cmnKrfCreate(2); + assertEquals(true, dif.hasKrf(2)); + dif.createRegion(drv); + dif.forceCompaction(); + dif.close(); + + dif = new DiskInitFile("testKrfIds", parent, true, Collections.<File>emptySet()); + assertEquals(true, dif.hasKrf(1)); + assertEquals(true, dif.hasKrf(2)); + dif.cmnCrfDelete(1); + assertEquals(false, dif.hasKrf(1)); + assertEquals(true, dif.hasKrf(2)); + dif.cmnCrfDelete(2); + assertEquals(false, dif.hasKrf(2)); + dif.createRegion(drv); + dif.forceCompaction(); + dif.close(); + + dif = new DiskInitFile("testKrfIds", parent, true, Collections.<File>emptySet()); + assertEquals(false, dif.hasKrf(1)); + assertEquals(false, dif.hasKrf(2)); + dif.destroy(); + } + }
