You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kh...@apache.org on 2017/03/17 20:11:24 UTC
[49/49] geode git commit: GEODE-2536: Remove the inappropriate
implementation of methods in DiskId for persistent regions.
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/26700b5d
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/26700b5d
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/26700b5d
Branch: refs/heads/feature/GEODE-2420
Commit: 26700b5d0df5468d58acbde6f82ca331544c760f
Parents: fca17e5
Author: eshu <es...@pivotal.io>
Authored: Thu Mar 16 15:07:38 2017 -0700
Committer: Ken Howe <kh...@pivotal.io>
Committed: Fri Mar 17 13:09:46 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/26700b5d/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/26700b5d/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/26700b5d/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/26700b5d/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);
+ }
+
}