geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From onich...@apache.org
Subject [geode] 02/02: GEODE-7085: Ensure bitset is flushed in all code paths
Date Mon, 26 Aug 2019 17:51:56 GMT
This is an automated email from the ASF dual-hosted git repository.

onichols pushed a commit to branch release/1.10.0
in repository https://gitbox.apache.org/repos/asf/geode.git

commit e341c210005d0925b686c00f29b80e50b132ef5a
Author: Dan Smith <upthewaterspout@apache.org>
AuthorDate: Mon Aug 19 16:40:57 2019 -0700

    GEODE-7085: Ensure bitset is flushed in all code paths
    
    In recordVersion, there was a code path where we would not call
    flushBitSetDuringRecording before calling setVersionInBitSet. Moving the
    flush to the top of the method to ensure that the bit set is always
    flushed, and we never end up with a case where we try to set too large
    of a bit.
    
    By code inspection, we determined that initializeFrom was what got us
    into the state that triggered this code path - it could leave
    bitSetVersion at a small number while setting version to a large number.
    Fixing initializeFrom so that it correctly sets bitSetVersion.
    
    Co-Authored-By: Ernest Burghardt <eburghardt@pivotal.io>
    (cherry picked from commit f17931bf541fc0255112f713931388d9ee0bbc30)
---
 .../cache/versions/RegionVersionHolder.java        | 22 +++++++++++--------
 .../RegionVersionHolderBitSetJUnitTest.java        | 25 ++++++++++++++++++++++
 .../versions/RegionVersionHolderJUnitTest.java     |  8 +++----
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
index 7102943..321bf63 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
@@ -345,6 +345,9 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable
{
   }
 
   private void recordVersionWithBitSet(long version) {
+
+    flushBitSetDuringRecording(version);
+
     if (this.version == version) {
       if (version >= this.bitSetVersion) {
         setVersionInBitSet(version);
@@ -353,7 +356,6 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable
{
       return;
     }
 
-    flushBitSetDuringRecording(version);
 
     if (version >= this.bitSetVersion) {
       if (this.getSpecialException() != null) {
@@ -443,14 +445,7 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable
{
     this.exceptions = other.exceptions;
     this.version = other.version;
 
-    // Initialize the bit set to be empty. Merge bit set should
-    // have already done this, but just to be sure.
-    if (this.bitSet != null) {
-      this.bitSetVersion = this.version;
-      // Make sure the bit set is empty except for the first, bit, indicating
-      // that the version has been received.
-      this.bitSet.set(0);
-    }
+
 
     // Now if this.version/exceptions overlap with myVersion/myExceptions, use this'
     // The only case needs special handling is: if myVersion is newer than this.version,
@@ -471,6 +466,15 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable
{
       this.exceptions.add(i, e);
       this.version = myVersion;
     }
+
+    // Initialize the bit set to be empty. Merge bit set should
+    // have already done this, but just to be sure.
+    if (this.bitSet != null) {
+      this.bitSetVersion = this.version;
+      // Make sure the bit set is empty except for the first, bit, indicating
+      // that the version has been received.
+      this.bitSet.set(0);
+    }
   }
 
   /**
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderBitSetJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderBitSetJUnitTest.java
index b583b51..e96b275 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderBitSetJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderBitSetJUnitTest.java
@@ -43,6 +43,31 @@ public class RegionVersionHolderBitSetJUnitTest {
   }
 
   @Test
+  public void initializeFromUpdatesBitSetVersionCorrectly() {
+    RegionVersionHolder holder = createHolder(true);
+    RegionVersionHolder other = createHolder(true);
+
+    int moreThanBitSetWidth = BIT_SET_WIDTH + 10;
+    holder.recordVersion(moreThanBitSetWidth);
+    other.recordVersion(2);
+
+    holder.initializeFrom(other);
+
+    assertThat(holder.getBitSetVersionForTesting()).isEqualTo(moreThanBitSetWidth);
+    assertEquals(bitSet(0), holder.getBitSetForTesting());
+    assertHasExceptions(holder, RVVException.createException(2, moreThanBitSetWidth + 1),
+        RVVException.createException(0, 2));
+
+    holder.recordVersion(moreThanBitSetWidth);
+
+    assertThat(holder.getBitSetVersionForTesting()).isEqualTo(moreThanBitSetWidth);
+    assertEquals(bitSet(0), holder.getBitSetForTesting());
+    assertContains(holder, 2, moreThanBitSetWidth);
+    assertHasExceptions(holder, RVVException.createException(2, moreThanBitSetWidth),
+        RVVException.createException(0, 2));
+  }
+
+  @Test
   public void recordVersionLessThanBitSetWidthShouldNotMoveBitSet() {
     RegionVersionHolder h = createHolder(true);
     int version = BIT_SET_WIDTH - 10;
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
index d40398b..0d1e385 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
@@ -1580,10 +1580,10 @@ public class RegionVersionHolderJUnitTest {
         assertEquals(100, vh4.getVersion());
         compareWithBitSet(bs1, vh4);
 
-        // use vh1 to overwrite vh2
-        vh1.version = 105;
-        vh1.addException(100, 106);
-        assertTrue(vh2.sameAs(vh1));
+        // Make sure vh2 is still valid after the clone() call
+        assertEquals(105, vh2.version);
+        assertEquals(100, vh2.getVersion());
+        compareWithBitSet(bs1, vh2);
         validateExceptions(vh2);
       }
     } finally {


Mime
View raw message