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 <upthewatersp...@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 <eburgha...@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 {
