You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by on...@apache.org on 2019/08/26 17:51:54 UTC
[geode] branch release/1.10.0 updated (a0b20b8 -> e341c21)
This is an automated email from the ASF dual-hosted git repository.
onichols pushed a change to branch release/1.10.0
in repository https://gitbox.apache.org/repos/asf/geode.git.
from a0b20b8 Add 1.9.1 to Version list
new 29120de GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH (#3922)
new e341c21 GEODE-7085: Ensure bitset is flushed in all code paths
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../cache/versions/BitSetExceptionIterator.java | 91 ++++++++++
.../internal/cache/versions/RVVException.java | 6 +
.../cache/versions/RegionVersionHolder.java | 143 ++++++++--------
.../versions/BitSetExceptionIteratorTest.java | 89 ++++++++++
.../versions/RegionVersionHolder2JUnitTest.java | 8 +-
.../RegionVersionHolderBitSetJUnitTest.java | 190 +++++++++++++++++++++
.../versions/RegionVersionHolderJUnitTest.java | 8 +-
.../versions/RegionVersionHolderUtilities.java | 28 +--
.../cache/versions/RegionVersionVectorTest.java | 48 +++++-
9 files changed, 510 insertions(+), 101 deletions(-)
create mode 100644 geode-core/src/main/java/org/apache/geode/internal/cache/versions/BitSetExceptionIterator.java
create mode 100644 geode-core/src/test/java/org/apache/geode/internal/cache/versions/BitSetExceptionIteratorTest.java
create mode 100644 geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderBitSetJUnitTest.java
copy geode-junit/src/main/java/org/apache/geode/test/junit/assertions/TabularResultModelRowAssert.java => geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderUtilities.java (54%)
[geode] 01/02: GEODE-7085: Ensuring the bitset stays within
BIT_SET_WIDTH (#3922)
Posted by on...@apache.org.
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 29120def0c006344b5cd3f15ae9d72d1a84b4566
Author: Dan Smith <up...@apache.org>
AuthorDate: Thu Aug 15 10:05:29 2019 -0700
GEODE-7085: Ensuring the bitset stays within BIT_SET_WIDTH (#3922)
Ensuring that when we call recordVersion on a RegionVersionHolder,
we appropriately move the bitSet to match the new version we are
recording, rather than trying to expand it. In particular, if new
version is greater than Integer.MAX_VALUE, we can't record than in out
integer indexed bit set.
This change rewrites addBitSetExceptions. The logic is now broken into a
BitSetExceptionIterator, which converts some or all of the bit set into
RVVException objects, and the logic to slide the bit set forward to a
new bitSetVersion.
Adding unit tests that show that large versions cause an
IndexOutOfBounds exception from recordGCVersion. Adding more unit tests
for the internal state of the bitset.
(cherry picked from commit f58710116db1cd8c509b59a43ffa050a073234d7)
---
.../cache/versions/BitSetExceptionIterator.java | 91 ++++++++++++
.../internal/cache/versions/RVVException.java | 6 +
.../cache/versions/RegionVersionHolder.java | 121 +++++++--------
.../versions/BitSetExceptionIteratorTest.java | 89 +++++++++++
.../versions/RegionVersionHolder2JUnitTest.java | 8 +-
.../RegionVersionHolderBitSetJUnitTest.java | 165 +++++++++++++++++++++
.../versions/RegionVersionHolderUtilities.java | 37 +++++
.../cache/versions/RegionVersionVectorTest.java | 48 +++++-
8 files changed, 490 insertions(+), 75 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/BitSetExceptionIterator.java b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/BitSetExceptionIterator.java
new file mode 100644
index 0000000..d880e9e
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/BitSetExceptionIterator.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.versions;
+
+import java.util.BitSet;
+import java.util.Iterator;
+
+/**
+ * An iterator on a bit set that produces {@link RVVException} for gaps
+ * in the set bits in the bitset.
+ */
+public class BitSetExceptionIterator implements Iterator<RVVException> {
+ private final BitSet bitSet;
+ private long bitSetVersion;
+ private final long maximumVersion;
+ private long nextClearBit;
+
+ /**
+ * Create a new bitset iterator
+ *
+ * @param bitSet The bitset to iterate on
+ * @param bitSetVersion An offset of the bitset. If this iterator generates a RVVException,
+ * the previous and next values will include this base value.
+ * @param maximumVersion The value to stop creating exceptions at. This should be greater than
+ * bitSetVersion. This may be within the bitSetVersion+bitSet.size, or it could be much
+ * large.
+ * If this value falls within the bitset, gaps past this value will not be returned as
+ * RVVExceptions
+ * by this iterator.
+ */
+ public BitSetExceptionIterator(BitSet bitSet, long bitSetVersion, long maximumVersion) {
+ this.bitSet = bitSet;
+ this.bitSetVersion = bitSetVersion;
+ this.maximumVersion = maximumVersion;
+ this.nextClearBit = findNextClearBit(bitSet, 0);
+ }
+
+ /**
+ * Find the next clear bit from a given index in the bitset, that is less than or
+ * equal to our maximum version for this iterator.
+ *
+ * @return the next clear bit, or -1 if there is no next clear bit within the range.
+ */
+ private int findNextClearBit(BitSet bitSet, int fromIndex) {
+ int nextClearBit = bitSet.nextClearBit(fromIndex);
+
+ long maxmimumClearBit = maximumVersion - bitSetVersion;
+ if (nextClearBit >= maxmimumClearBit) {
+ // We found empty bits, but past the offset we are interested in
+ // Ignore these
+ return -1;
+ }
+
+ return nextClearBit;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return nextClearBit != -1;
+ }
+
+ @Override
+ public RVVException next() {
+ if (!hasNext()) {
+ return null;
+ }
+
+ int nextSetBit = bitSet.nextSetBit((int) Math.min(Integer.MAX_VALUE, nextClearBit));
+
+ long nextSetVersion = nextSetBit == -1 ? maximumVersion : nextSetBit + bitSetVersion;
+
+ RVVException exception =
+ RVVException.createException(nextClearBit + bitSetVersion - 1, nextSetVersion);
+
+ nextClearBit = nextSetBit == -1 ? -1 : findNextClearBit(bitSet, nextSetBit);
+
+ return exception;
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RVVException.java b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RVVException.java
index 16bb652..b47b58b 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RVVException.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RVVException.java
@@ -71,6 +71,12 @@ abstract class RVVException
long nextVersion;
+ /**
+ * Create an exception to represent missing versions
+ *
+ * @param previousVersion The previous version before the first missing version
+ * @param nextVersion The next received version after the last missing version
+ */
static RVVException createException(long previousVersion, long nextVersion) {
return createException(previousVersion, nextVersion, 0);
}
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 491b597..7102943 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
@@ -29,6 +29,7 @@ import org.apache.logging.log4j.Logger;
import org.apache.geode.DataSerializable;
import org.apache.geode.annotations.Immutable;
import org.apache.geode.annotations.internal.MutableForTesting;
+import org.apache.geode.internal.Assert;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.log4j.LogMarker;
@@ -136,6 +137,10 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
return this.bitSetVersion;
}
+ public BitSet getBitSetForTesting() {
+ return this.bitSet;
+ }
+
private synchronized List<RVVException> getExceptions() {
mergeBitSet();
if (this.exceptions != null) {
@@ -157,7 +162,10 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
return getExceptions().toString();
}
- public void setVersion(long ver) {
+ /**
+ * Should only be called as part of cloning a RegionVersionHolder
+ */
+ void setVersion(long ver) {
this.version = ver;
}
@@ -242,8 +250,11 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
return; // it fits in this bitset
}
- int length = BIT_SET_WIDTH;
- int bitCountToFlush = length * 3 / 4;
+ int bitCountToFlush = BIT_SET_WIDTH * 3 / 4;
+
+ // We can only flush up to the last set bit because
+ // the exceptions list includes a "next version" that indicates a received version.
+ bitCountToFlush = bitSet.previousSetBit(bitCountToFlush);
if (logger.isTraceEnabled(LogMarker.RVV_VERBOSE)) {
logger.trace(LogMarker.RVV_VERBOSE, "flushing RVV bitset bitSetVersion={}; bits={}",
@@ -251,13 +262,13 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
}
// see if we can shift part of the bits so that exceptions in the recent bits can
// be kept in the bitset and later filled without having to create real exception objects
- if (version >= this.bitSetVersion + length + bitCountToFlush) {
+ if (bitCountToFlush == -1 || version >= this.bitSetVersion + BIT_SET_WIDTH + bitCountToFlush) {
// nope - flush the whole bitset
- addBitSetExceptions(length, version);
+ addBitSetExceptions(version);
} else {
// yes - flush the lower part. We can only flush up to the last set bit because
// the exceptions list includes a "next version" that indicates a received version.
- addBitSetExceptions(bitCountToFlush, this.bitSetVersion + bitCountToFlush);
+ addBitSetExceptions(this.bitSetVersion + bitCountToFlush);
}
if (logger.isTraceEnabled(LogMarker.RVV_VERBOSE)) {
logger.trace(LogMarker.RVV_VERBOSE, "After flushing bitSetVersion={}; bits={}",
@@ -269,7 +280,7 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
/** merge bit-set exceptions into the regular exceptions list */
private synchronized void mergeBitSet() {
if (this.bitSet != null && this.bitSetVersion < this.version) {
- addBitSetExceptions((int) (this.version - this.bitSetVersion), this.version);
+ addBitSetExceptions(this.version);
}
}
@@ -280,63 +291,36 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
* adjusted and a new bitSetVersion is established.
*
* @param newVersion the desired new bitSetVersion, which may be > the max representable in the
- * bitset
- * @param numBits the desired number of bits to flush from the bitset
+ * bitset. This should *always* be a version that has been received, because this
+ * method may need to create an exception up to this version, and the existance of an
+ * exception implies that the final version was received.
+ *
+ *
*/
- private void addBitSetExceptions(int numBits, long newVersion) {
- final boolean isDebugEnabled_RVV = logger.isTraceEnabled(LogMarker.RVV_VERBOSE);
- int lastSetIndex = -1;
-
- if (isDebugEnabled_RVV) {
- logger.trace(LogMarker.RVV_VERBOSE, "addBitSetExceptions({},{})", numBits, newVersion);
+ private void addBitSetExceptions(long newVersion) {
+ if (newVersion <= bitSetVersion) {
+ return;
}
- for (int idx = 0; idx < numBits;) {
- int nextMissingIndex = this.bitSet.nextClearBit(idx);
- if (nextMissingIndex < 0) {
- break;
- }
-
- lastSetIndex = nextMissingIndex - 1;
-
- int nextReceivedIndex = this.bitSet.nextSetBit(nextMissingIndex + 1);
- long nextReceivedVersion = -1;
- if (nextReceivedIndex > 0) {
- lastSetIndex = nextReceivedIndex;
- nextReceivedVersion = (long) (nextReceivedIndex) + this.bitSetVersion;
- idx = nextReceivedIndex + 1;
- if (isDebugEnabled_RVV) {
- logger.trace(LogMarker.RVV_VERBOSE,
- "found gap in bitSet: missing bit at index={}; next set index={}", nextMissingIndex,
- nextReceivedIndex);
- }
- } else {
- // We can't flush any more bits from the bit set because there
- // are no more received versions
- if (isDebugEnabled_RVV) {
- logger.trace(LogMarker.RVV_VERBOSE,
- "terminating flush at bit {} because of missing entries", lastSetIndex);
- }
- this.bitSetVersion += lastSetIndex;
- this.bitSet.clear();
- if (lastSetIndex != -1) {
- this.bitSet.set(0);
- }
- return;
- }
- long nextMissingVersion = Math.max(1, nextMissingIndex + this.bitSetVersion);
- if (nextReceivedVersion > nextMissingVersion) {
- addException(nextMissingVersion - 1, nextReceivedVersion);
- if (isDebugEnabled_RVV) {
- logger.trace(LogMarker.RVV_VERBOSE, "Added rvv exception e<rv{} - rv{}>",
- (nextMissingVersion - 1), nextReceivedVersion);
- }
- }
+ // Add all of the exceptions that should be flushed from the bitset as real exceptions
+ Iterator<RVVException> exceptionIterator =
+ new BitSetExceptionIterator(bitSet, bitSetVersion, newVersion);
+ while (exceptionIterator.hasNext()) {
+ addException(exceptionIterator.next());
}
- this.bitSet = this.bitSet.get(lastSetIndex, Math.max(lastSetIndex + 1, bitSet.size()));
- if (lastSetIndex > 0) {
- this.bitSetVersion = this.bitSetVersion + (long) lastSetIndex;
+
+ // Move the data in the bitset forward to reflect the new version
+ if (newVersion > bitSetVersion + bitSet.size()) {
+ // Optimization - if the new version is past the end of the bitset, just clear the bitset
+ bitSet.clear();
+ } else {
+ // Otherwise slide the bitset over to the new offset
+ int offsetIncrease = (int) (newVersion - bitSetVersion);
+ bitSet = bitSet.get(offsetIncrease, bitSet.size());
}
+
+ // Move the bitset version
+ bitSetVersion = newVersion;
}
synchronized void recordVersion(long version) {
@@ -384,7 +368,11 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
}
private void setVersionInBitSet(long version) {
- this.bitSet.set((int) (version - this.bitSetVersion));
+ long bitToSet = version - this.bitSetVersion;
+ if (bitToSet > BIT_SET_WIDTH) {
+ Assert.fail("Trying to set a bit larger than the size of the bitset " + bitToSet);
+ }
+ this.bitSet.set(Math.toIntExact(bitToSet));
}
private void logRecordVersion(long version) {
@@ -398,20 +386,25 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
/**
* Add an exception that is older than this.bitSetVersion.
*/
- synchronized void addException(long previousVersion, long nextVersion) {
+ synchronized void addException(final long previousVersion, final long nextVersion) {
+ RVVException newException = RVVException.createException(previousVersion, nextVersion);
+ addException(newException);
+ }
+
+ private void addException(RVVException newException) {
if (this.exceptions == null) {
this.exceptions = new LinkedList<RVVException>();
}
int i = 0;
for (Iterator<RVVException> it = this.exceptions.iterator(); it.hasNext(); i++) {
RVVException e = it.next();
- if (previousVersion >= e.nextVersion) {
- RVVException except = RVVException.createException(previousVersion, nextVersion);
+ if (newException.previousVersion >= e.nextVersion) {
+ RVVException except = newException;
this.exceptions.add(i, except);
return;
}
}
- this.exceptions.add(RVVException.createException(previousVersion, nextVersion));
+ this.exceptions.add(newException);
}
synchronized void removeExceptionsOlderThan(long v) {
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/BitSetExceptionIteratorTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/BitSetExceptionIteratorTest.java
new file mode 100644
index 0000000..a54766d
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/BitSetExceptionIteratorTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.versions;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import org.junit.Test;
+
+public class BitSetExceptionIteratorTest {
+
+ @Test
+ public void findsLeadingException() {
+ BitSet bitSet = new BitSet(10);
+ bitSet.set(1, 10);
+
+ BitSetExceptionIterator exceptionIterator = new BitSetExceptionIterator(bitSet, 50, 55);
+ assertContainsExceptions(exceptionIterator, RVVException.createException(49, 51));
+ }
+
+
+ @Test
+ public void findsTrailingException() {
+ BitSet bitSet = new BitSet(10);
+ bitSet.set(0, 6);
+
+ BitSetExceptionIterator exceptionIterator = new BitSetExceptionIterator(bitSet, 50, 57);
+ assertContainsExceptions(exceptionIterator, RVVException.createException(55, 57));
+
+ }
+
+ @Test
+ public void findsTrailingExceptionDueToLargeVersion() {
+ BitSet bitSet = new BitSet(10);
+ bitSet.set(0, 10);
+
+ BitSetExceptionIterator exceptionIterator = new BitSetExceptionIterator(bitSet, 50, 61);
+ assertContainsExceptions(exceptionIterator, RVVException.createException(59, 61));
+ }
+
+ @Test
+ public void ignoresExceptionsPastEndVersion() {
+ BitSet bitSet = new BitSet(10);
+ bitSet.set(0, 8);
+
+ BitSetExceptionIterator exceptionIterator = new BitSetExceptionIterator(bitSet, 50, 57);
+ assertContainsExceptions(exceptionIterator);
+ }
+
+ @Test
+ public void ignoresExceptionsIfNextVersionIsOnePastTheEndOfFullBitset() {
+ BitSet bitSet = new BitSet(10);
+ bitSet.set(0, 10);
+
+ BitSetExceptionIterator exceptionIterator = new BitSetExceptionIterator(bitSet, 50, 60);
+ assertContainsExceptions(exceptionIterator);
+ }
+
+ @Test
+ public void findsMiddleException() {
+ BitSet bitSet = new BitSet(10);
+ bitSet.set(0, 4);
+ bitSet.set(6, 10);
+
+ BitSetExceptionIterator exceptionIterator = new BitSetExceptionIterator(bitSet, 50, 59);
+ assertContainsExceptions(exceptionIterator, RVVException.createException(53, 56));
+
+ }
+
+ private void assertContainsExceptions(BitSetExceptionIterator exceptionIterator,
+ RVVException... expectedExceptions) {
+ List<RVVException> foundExceptions = new ArrayList<>();
+ exceptionIterator.forEachRemaining(foundExceptions::add);
+ RegionVersionHolderUtilities.assertSameExceptions(foundExceptions, expectedExceptions);
+ }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolder2JUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolder2JUnitTest.java
index 97189fa..0ace35b 100755
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolder2JUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolder2JUnitTest.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.internal.cache.versions;
+import static org.apache.geode.internal.cache.versions.RegionVersionHolder.BIT_SET_WIDTH;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
@@ -122,7 +123,7 @@ public class RegionVersionHolder2JUnitTest {
private void fillLargeException(boolean useBitSet) {
RegionVersionHolder h = createHolder(useBitSet);
- long bigVersion = RegionVersionHolder.BIT_SET_WIDTH + 1;
+ long bigVersion = BIT_SET_WIDTH + 1;
h.recordVersion(bigVersion);
for (long i = 0l; i < bigVersion; i++) {
h.recordVersion(i);
@@ -134,7 +135,7 @@ public class RegionVersionHolder2JUnitTest {
@Test
public void testReceiveDuplicateAfterBitSetFlushWithBitSet() {
RegionVersionHolder h = createHolder(true);
- long bigVersion = RegionVersionHolder.BIT_SET_WIDTH + 1;
+ long bigVersion = BIT_SET_WIDTH + 1;
h.recordVersion(bigVersion);
for (long i = 0l; i < bigVersion; i++) {
h.recordVersion(i);
@@ -157,11 +158,12 @@ public class RegionVersionHolder2JUnitTest {
// assertIndexDetailsEquals("unexpected RVV exception : " + h, 0, h.getExceptionCount());
}
+
private void createSpecialException(RegionVersionHolder h) {
h.addException(h.getVersion() - 1, h.getVersion() + 1);
}
- private RegionVersionHolder createHolder(boolean useBitSet) {
+ static RegionVersionHolder createHolder(boolean useBitSet) {
if (useBitSet) {
return new RegionVersionHolder("id");
}
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
new file mode 100644
index 0000000..b583b51
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderBitSetJUnitTest.java
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.versions;
+
+import static org.apache.geode.internal.cache.versions.RegionVersionHolder.BIT_SET_WIDTH;
+import static org.apache.geode.internal.cache.versions.RegionVersionHolder2JUnitTest.createHolder;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import java.util.BitSet;
+import java.util.List;
+import java.util.stream.IntStream;
+import java.util.stream.LongStream;
+
+import org.junit.Test;
+
+/**
+ * Tests of how the RegionVersionHolder maintains it's bitset under updates
+ */
+public class RegionVersionHolderBitSetJUnitTest {
+
+ @Test
+ public void recordVersionLargerThanIntMaxValueShouldSucceed() {
+ RegionVersionHolder h = createHolder(true);
+ long version = ((long) Integer.MAX_VALUE) + 10L;
+ h.recordVersion(version);
+ assertEquals(version, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(0), h.getBitSetForTesting());
+ assertContains(h, version);
+ assertHasExceptions(h, RVVException.createException(0, version));
+ }
+
+ @Test
+ public void recordVersionLessThanBitSetWidthShouldNotMoveBitSet() {
+ RegionVersionHolder h = createHolder(true);
+ int version = BIT_SET_WIDTH - 10;
+ h.recordVersion(version);
+ assertEquals(1, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(version - 1), h.getBitSetForTesting());
+ assertContains(h, version);
+ assertHasExceptions(h, RVVException.createException(0, version));
+ }
+
+ @Test
+ public void recordVersionGreaterThanBitSetWidthShouldMoveBitSet() {
+ RegionVersionHolder h = createHolder(true);
+ long version = ((long) Integer.MAX_VALUE) - 10L;
+ h.recordVersion(version);
+ assertEquals(version, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(0), h.getBitSetForTesting());
+ assertContains(h, version);
+ assertHasExceptions(h, RVVException.createException(0, version));
+ }
+
+ @Test
+ public void recordFirstVersionShouldSetFirstBit() {
+ RegionVersionHolder h = createHolder(true);
+ h.recordVersion(1);
+ assertEquals(1, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(0), h.getBitSetForTesting());
+ assertContains(h, 1);
+ assertHasExceptions(h, new RVVException[0]);
+ }
+
+ @Test
+ public void recordLargeVersionWithSomeBitsSetShouldMoveBitSet() {
+ RegionVersionHolder h = createHolder(true);
+
+ LongStream.range(1, 10).forEach(h::recordVersion);
+ long version = ((long) Integer.MAX_VALUE) - 10L;
+ h.recordVersion(version);
+ assertEquals(version, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(0), h.getBitSetForTesting());
+ assertContains(h, 1, 2, 3, 4, 5, 6, 7, 8, 9, version);
+ assertHasExceptions(h, RVVException.createException(9, version));
+ }
+
+ @Test
+ public void recordOneVersionPastBitSetWidthShouldMoveBitSet() {
+ RegionVersionHolder h = createHolder(true);
+
+ LongStream.range(1, BIT_SET_WIDTH + 1).forEach(h::recordVersion);
+ assertEquals(1, h.getBitSetVersionForTesting());
+ BitSet expectedBitSet = new BitSet(BIT_SET_WIDTH);
+ expectedBitSet.set(0, BIT_SET_WIDTH);
+ assertEquals(expectedBitSet, h.getBitSetForTesting());
+
+ h.recordVersion(BIT_SET_WIDTH + 1);
+ assertEquals(BIT_SET_WIDTH * 3 / 4 + 1, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(IntStream.range(0, (BIT_SET_WIDTH / 4) + 1).toArray()),
+ h.getBitSetForTesting());
+ assertContains(h, LongStream.range(0, BIT_SET_WIDTH + 2).toArray());
+ assertHasExceptions(h, new RVVException[0]);
+ }
+
+ @Test
+ public void recordVersionGreaterThanTwiceBitSetWidthShouldMoveBitSetAndCreateExceptions() {
+ RegionVersionHolder h = createHolder(true);
+ long version = ((long) Integer.MAX_VALUE) - 10L;
+ LongStream.range(1, 10).forEach(h::recordVersion);
+ h.recordVersion(15);
+ h.recordVersion(version);
+ assertEquals(version, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(0), h.getBitSetForTesting());
+ assertContains(h, 1, 2, 3, 4, 5, 6, 7, 8, 9, 15, version);
+ assertHasExceptions(h,
+ RVVException.createException(15, version),
+ RVVException.createException(9, 15));
+ }
+
+ @Test
+ public void recordVersionLessThanTwiceBitSetWidthShouldSlideBitSet() {
+ RegionVersionHolder h = createHolder(true);
+ LongStream.range(1, 10).forEach(h::recordVersion);
+ h.recordVersion(15);
+ int lessThanBitSetVersion = BIT_SET_WIDTH - 10;
+ h.recordVersion(lessThanBitSetVersion);
+ int moreThanBitSetVersion = BIT_SET_WIDTH + 10;
+ h.recordVersion(moreThanBitSetVersion);
+
+ // The bit set will only slide to start from 15, because that is the last set bit
+ // less than 3/4 the size of the bit set
+ assertEquals(15, h.getBitSetVersionForTesting());
+ assertEquals(bitSet(0, lessThanBitSetVersion - 15, moreThanBitSetVersion - 15),
+ h.getBitSetForTesting());
+
+ assertContains(h, 1, 2, 3, 4, 5, 6, 7, 8, 9, 15, lessThanBitSetVersion, moreThanBitSetVersion);
+ assertHasExceptions(h,
+ RVVException.createException(lessThanBitSetVersion, moreThanBitSetVersion),
+ RVVException.createException(15, lessThanBitSetVersion),
+ RVVException.createException(9, 15));
+ }
+
+
+ private void assertContains(RegionVersionHolder h, long... versions) {
+ LongStream.of(versions).forEach(version -> {
+ assertThat(h.contains(version))
+ .withFailMessage("Did not contain %s", version)
+ .isTrue();
+ });
+ }
+
+ private void assertHasExceptions(RegionVersionHolder h, RVVException... exceptions) {
+ List<RVVException> actualExceptions = h.getExceptionForTest();
+ RegionVersionHolderUtilities.assertSameExceptions(actualExceptions, exceptions);
+ }
+
+ private BitSet bitSet(int... setBits) {
+ BitSet bitSet = new BitSet(BIT_SET_WIDTH);
+ IntStream.of(setBits).forEach(bitSet::set);
+ return bitSet;
+ }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderUtilities.java b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderUtilities.java
new file mode 100644
index 0000000..01e048a
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderUtilities.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.versions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.List;
+
+public class RegionVersionHolderUtilities {
+ static void assertSameExceptions(List<RVVException> actualExceptions,
+ RVVException[] exceptions) {
+ List<RVVException> expectedExceptions = Arrays.asList(exceptions);
+ assertThat(actualExceptions)
+ .withFailMessage("Expected exceptions %s but got %s", expectedExceptions, actualExceptions)
+ .hasSize(exceptions.length);
+
+ for (int i = 0; i < exceptions.length; i++) {
+ assertThat(actualExceptions.get(i).sameAs(expectedExceptions.get(i)))
+ .withFailMessage("Expected exceptions %s but got %s", expectedExceptions,
+ actualExceptions)
+ .isTrue();
+ }
+ }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java
index ce328ca..5141122 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java
@@ -675,6 +675,34 @@ public class RegionVersionVectorTest {
assertFalse(holder1.contains(4));
}
+
+ @Test
+ public void recordLargeGCVersionShouldRecordSuccessfully() {
+ DiskStoreID id0 = new DiskStoreID(0, 0);
+ DiskStoreID id1 = new DiskStoreID(0, 1);
+
+ DiskRegionVersionVector rvv0 = new DiskRegionVersionVector(id0);
+
+ rvv0.recordGCVersion(id1, ((long) Integer.MAX_VALUE) - 10L);
+ }
+
+ @Test
+ public void incrementingTheLocalVersionShouldNotCreateExceptions() {
+ DiskStoreID id0 = new DiskStoreID(0, 0);
+ DiskStoreID id1 = new DiskStoreID(0, 1);
+
+ DiskRegionVersionVector rvv0 = new DiskRegionVersionVector(id0);
+
+ // Increment the version a couple of times
+ rvv0.getNextVersion();
+ rvv0.getNextVersion();
+
+ RegionVersionVector<DiskStoreID> clone = rvv0.getCloneForTransmission();
+
+ assertThat(clone.getHolderForMember(id0).getExceptionForTest()).isEmpty();
+ assertThat(rvv0.getHolderForMember(id0).getExceptionForTest()).isEmpty();
+ }
+
@Test
public void testRemoveOldVersions() {
DiskStoreID id0 = new DiskStoreID(0, 0);
@@ -832,7 +860,8 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(lostMember);
- regionVersionHolder.setVersion(2);
+ regionVersionHolder.recordVersion(1);
+ regionVersionHolder.recordVersion(2);
memberToRegionVersionHolder.put(lostMember, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(lostMember, memberToRegionVersionHolder,
@@ -854,7 +883,6 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(lostMember);
- regionVersionHolder.setVersion(0);
memberToRegionVersionHolder.put(lostMember, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(lostMember, memberToRegionVersionHolder,
@@ -876,7 +904,6 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
- regionVersionHolder.setVersion(0);
// memberToRegionVersionHolder.put(provider, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(requester, memberToRegionVersionHolder,
@@ -898,7 +925,6 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
- regionVersionHolder.setVersion(0);
memberToRegionVersionHolder.put(provider, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(requester, memberToRegionVersionHolder,
@@ -922,7 +948,8 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
- regionVersionHolder.setVersion(2);
+ regionVersionHolder.recordVersion(1);
+ regionVersionHolder.recordVersion(2);
memberToRegionVersionHolder.put(provider, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(requester, memberToRegionVersionHolder,
@@ -945,7 +972,8 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
- regionVersionHolder.setVersion(2);
+ regionVersionHolder.recordVersion(1);
+ regionVersionHolder.recordVersion(2);
memberToRegionVersionHolder.put(provider, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(requester, memberToRegionVersionHolder,
@@ -963,7 +991,10 @@ public class RegionVersionVectorTest {
new ConcurrentHashMap<>();
memberToGcVersion.put(requester, new Long(3));
RegionVersionHolder pRegionVersionHolder = new RegionVersionHolder(provider);
- pRegionVersionHolder.setVersion(4);
+ pRegionVersionHolder.recordVersion(1);
+ pRegionVersionHolder.recordVersion(2);
+ pRegionVersionHolder.recordVersion(3);
+ pRegionVersionHolder.recordVersion(4);
RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
1, memberToGcVersion, 1, false, pRegionVersionHolder);
@@ -971,7 +1002,8 @@ public class RegionVersionVectorTest {
ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
new ConcurrentHashMap<>();
RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
- regionVersionHolder.setVersion(2);
+ regionVersionHolder.recordVersion(1);
+ regionVersionHolder.recordVersion(2);
memberToRegionVersionHolder.put(provider, regionVersionHolder);
RegionVersionVector requesterRvv =
new VMRegionVersionVector(requester, memberToRegionVersionHolder,
[geode] 02/02: GEODE-7085: Ensure bitset is flushed in all code
paths
Posted by on...@apache.org.
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 <up...@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 <eb...@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 {