geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From esh...@apache.org
Subject geode git commit: GEODE-2536: Remove the inappropriate implementation of methods in DiskId for persistent regions.
Date Thu, 16 Mar 2017 22:11:20 GMT
Repository: geode
Updated Branches:
  refs/heads/develop 426c2fcd3 -> 0c85fd656


GEODE-2536: Remove the inappropriate implementation of methods in DiskId for persistent regions.

markForWriting and unmarkForWriting should not be used for persistent region.
needsToBeWritten always return false now for persistent region, as the diskEntry either is
being written to disk (sync) or sheduled to be written to disk (async)


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/0c85fd65
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/0c85fd65
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/0c85fd65

Branch: refs/heads/develop
Commit: 0c85fd656ad1314397a871dc707cd0382632f9b7
Parents: 426c2fc
Author: eshu <eshu@pivotal.io>
Authored: Thu Mar 16 15:07:38 2017 -0700
Committer: eshu <eshu@pivotal.io>
Committed: Thu Mar 16 15:10:58 2017 -0700

----------------------------------------------------------------------
 .../apache/geode/internal/cache/DiskEntry.java  |  1 +
 .../org/apache/geode/internal/cache/DiskId.java | 26 +++++---------------
 .../geode/internal/cache/DiskStoreImpl.java     |  3 +--
 .../geode/internal/cache/DiskIdJUnitTest.java   | 24 ++++++++++++++++++
 4 files changed, 32 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/0c85fd65/geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java
index bde6a32..d00f7a0 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskEntry.java
@@ -1690,6 +1690,7 @@ public interface DiskEntry extends RegionEntry {
 
       // Asif: This will convert the -ve OplogKeyId to positive as part of fixing
       // Bug # 39989
+      // GEODE-2535 fix should address the original #39989 negative keyId issue.
       did.unmarkForWriting();
 
       // System.out.println("DEBUG: removeFromDisk doing remove(" + id + ")");

http://git-wip-us.apache.org/repos/asf/geode/blob/0c85fd65/geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java
index e802ac9..8d2c675 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskId.java
@@ -558,24 +558,17 @@ public abstract class DiskId {
 
     @Override
     void markForWriting() {
-      if (this.keyId > DiskRegion.INVALID_ID) {
-        // Mark the id as needing to be written
-        // The disk remove that this section used to do caused bug 30961
-        this.setKeyId(-this.keyId);
-      }
+      throw new IllegalStateException("Should not be used for persistent region");
     }
 
     @Override
     void unmarkForWriting() {
-      if (this.keyId < DiskRegion.INVALID_ID) {
-        // Mark the id as NOT needing to be written
-        this.setKeyId(-this.keyId);
-      }
+      // Do nothing
     }
 
     @Override
     boolean needsToBeWritten() {
-      return this.keyId <= DiskRegion.INVALID_ID;
+      return false;
     }
 
     @Override
@@ -643,19 +636,12 @@ public abstract class DiskId {
 
     @Override
     void markForWriting() {
-      if (this.keyId > DiskRegion.INVALID_ID) {
-        // Mark the id as needing to be written
-        // The disk remove that this section used to do caused bug 30961
-        this.setKeyId(-this.keyId);
-      }
+      throw new IllegalStateException("Should not be used for persistent region");
     }
 
     @Override
     void unmarkForWriting() {
-      if (this.keyId < DiskRegion.INVALID_ID) {
-        // Mark the id as NOT needing to be written
-        this.setKeyId(-this.keyId);
-      }
+      // Do nothing
     }
 
     @Override
@@ -670,7 +656,7 @@ public abstract class DiskId {
 
     @Override
     boolean needsToBeWritten() {
-      return this.keyId <= DiskRegion.INVALID_ID;
+      return false;
     }
   }
   final protected static class PersistenceWithLongOffset extends PersistenceWithLongOffsetNoLL
{

http://git-wip-us.apache.org/repos/asf/geode/blob/0c85fd65/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
index cce8100..642eed3 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java
@@ -704,8 +704,7 @@ public class DiskStoreImpl implements DiskStore {
     DiskRegion dr = region.getDiskRegion();
     DiskId id = entry.getDiskId();
     if (dr.isBackup() && id.getKeyId() < 0) {
-      throw new IllegalArgumentException(
-          LocalizedStrings.DiskRegion_CANT_PUT_A_KEYVALUE_PAIR_WITH_ID_0.toLocalizedString(id));
+      id.setKeyId(-id.getKeyId());
     }
     long start = async ? this.stats.startFlush() : this.stats.startWrite();
     if (!async) {

http://git-wip-us.apache.org/repos/asf/geode/blob/0c85fd65/geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java
index 195c6a2..494ccff 100755
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/DiskIdJUnitTest.java
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.internal.cache;
 
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.Assert.*;
 
 import org.junit.Test;
@@ -219,4 +220,27 @@ public class DiskIdJUnitTest {
     return DiskId.createDiskId(1024, true /* is persistence type */, true);
   }
 
+  /**
+   * Tests unmarkForWrite for persistent region does not change keyId
+   */
+  @Test
+  public void testPersistUnmarkForWrite() {
+    DiskId diskId = getDiskId();
+    diskId.setKeyId(11);
+    diskId.unmarkForWriting();
+    long newKeyId = diskId.getKeyId();
+
+    assertEquals(11, newKeyId);
+  }
+
+  /**
+   * Tests markForWrite for persistent region failed
+   */
+  @Test
+  public void testPersistMarkForWrite() {
+    DiskId diskId = getDiskId();
+    diskId.setKeyId(11);
+    assertThatThrownBy(() -> diskId.markForWriting()).isInstanceOf(IllegalStateException.class);
+  }
+
 }


Mime
View raw message