You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by la...@apache.org on 2017/04/29 00:12:46 UTC
[16/18] geode git commit: GEODE-2801: change krfIds to be thread safe
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/7207aa45
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/7207aa45
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/7207aa45
Branch: refs/heads/feature/GEODE-2852
Commit: 7207aa45b7e4298d5f92a5ea7a57afd5510e1e6a
Parents: 84e36d5
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Wed Apr 19 17:09:05 2017 -0700
Committer: Lynn Hughes-Godfrey <lh...@pivotal.io>
Committed: Fri Apr 28 17:06:33 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/7207aa45/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/7207aa45/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/7207aa45/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();
+ }
+
}