GEODE-1252: modify bits field atomically - AtomicIntegerFieldUpdater now used to modify bits field - added unit test for bit methods - removed unused constructors
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/44289546 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/44289546 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/44289546 Branch: refs/heads/feature/GEODE-835 Commit: 442895462472c31fb3b9f37ae61f7fe510423bf5 Parents: 6523c97 Author: Darrel Schneider <dschnei...@pivotal.io> Authored: Wed May 11 14:38:09 2016 -0700 Committer: Darrel Schneider <dschnei...@pivotal.io> Committed: Mon May 16 11:24:13 2016 -0700 ---------------------------------------------------------------------- .../internal/cache/versions/VersionTag.java | 83 +++++++++++------- .../versions/AbstractVersionTagTestBase.java | 92 ++++++++++++++++++++ .../cache/versions/VMVersionTagTest.java | 32 +++++++ .../sanctionedDataSerializables.txt | 4 +- 4 files changed, 179 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/44289546/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/VersionTag.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/VersionTag.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/VersionTag.java index 60e4299..7910996 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/VersionTag.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/versions/VersionTag.java @@ -19,6 +19,7 @@ package com.gemstone.gemfire.internal.cache.versions; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import org.apache.logging.log4j.Logger; @@ -74,6 +75,7 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali private static final int BITS_TIMESTAMP_APPLIED = 0x20; private static final int BITS_ALLOWED_BY_RESOLVER = 0x40; + // Note: the only valid BITS_* are 0xFFFF. /** * the per-entry version number for the operation @@ -100,10 +102,19 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali */ private byte distributedSystemId; + // In GEODE-1252 we found that the bits field + // was concurrently modified by calls to + // setPreviousMemberID and setRecorded. + // So bits has been changed to volatile and + // all modification to it happens using AtomicIntegerFieldUpdater. + private static final AtomicIntegerFieldUpdater<VersionTag> bitsUpdater = + AtomicIntegerFieldUpdater.newUpdater(VersionTag.class, "bits"); /** * boolean bits + * Note: this is an int field so it has 32 bits BUT only the lower 16 bits are serialized. + * So all our code should treat this an an unsigned short field. */ - private int bits; + private volatile int bits; /** * the initiator of the operation. If null, the initiator was the sender @@ -128,7 +139,11 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali /** record that the timestamp from this tag was applied to the cache */ public void setTimeStampApplied(boolean isTimeStampUpdated) { - this.bits |= BITS_TIMESTAMP_APPLIED; + if (isTimeStampUpdated) { + setBits(BITS_TIMESTAMP_APPLIED); + } else { + clearBits(~BITS_TIMESTAMP_APPLIED); + } } /** @@ -152,9 +167,9 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali public void setIsGatewayTag(boolean isGateway) { if (isGateway) { - this.bits = this.bits | BITS_GATEWAY_TAG; + setBits(BITS_GATEWAY_TAG); } else { - this.bits = this.bits & ~BITS_GATEWAY_TAG; + clearBits(~BITS_GATEWAY_TAG); } } @@ -193,7 +208,7 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali * set that this tag has been recorded in a receiver's RVV */ public void setRecorded() { - this.bits |= BITS_RECORDED; + setBits(BITS_RECORDED); } /** @@ -236,7 +251,7 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali * @param previousMemberID the previousMemberID to set */ public void setPreviousMemberID(T previousMemberID) { - this.bits |= BITS_HAS_PREVIOUS_ID; + setBits(BITS_HAS_PREVIOUS_ID); this.previousMemberID = previousMemberID; } @@ -249,9 +264,9 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali */ public VersionTag setPosDup(boolean flag) { if (flag) { - this.bits |= BITS_POSDUP; + setBits(BITS_POSDUP); } else { - this.bits &= ~BITS_POSDUP; + clearBits(~BITS_POSDUP); } return this; } @@ -268,9 +283,9 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali */ public VersionTag setAllowedByResolver(boolean flag) { if (flag) { - this.bits |= BITS_ALLOWED_BY_RESOLVER; + setBits(BITS_ALLOWED_BY_RESOLVER); } else { - this.bits &= ~BITS_ALLOWED_BY_RESOLVER; + clearBits(~BITS_ALLOWED_BY_RESOLVER); } return this; } @@ -319,21 +334,6 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali return !(this.entryVersion == 0 && this.regionVersionHighBytes == 0 && this.regionVersionLowBytes == 0); } - public VersionTag() { - } - - /** - * creates a version tag for a WAN gateway event - * - * @param timestamp - * @param dsid - */ - public VersionTag(long timestamp, int dsid) { - this.timeStamp = timestamp; - this.distributedSystemId = (byte) (dsid & 0xFF); - this.bits = BITS_GATEWAY_TAG + BITS_IS_REMOTE_TAG; - } - public void toData(DataOutput out) throws IOException { toData(out, true); } @@ -386,7 +386,7 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali if (logger.isTraceEnabled(LogMarker.VERSION_TAG)) { logger.info(LogMarker.VERSION_TAG, "deserializing {} with flags 0x{}", this.getClass(), Integer.toHexString(flags)); } - this.bits = in.readUnsignedShort(); + bitsUpdater.set(this, in.readUnsignedShort()); this.distributedSystemId = in.readByte(); if ((flags & VERSION_TWO_BYTES) != 0) { this.entryVersion = in.readShort() & 0xffff; @@ -408,11 +408,11 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali this.previousMemberID = readMember(in); } } - this.bits |= BITS_IS_REMOTE_TAG; + setIsRemoteForTesting(); } public void setIsRemoteForTesting() { - this.bits |= BITS_IS_REMOTE_TAG; + setBits(BITS_IS_REMOTE_TAG); } public abstract T readMember(DataInput in) throws IOException, ClassNotFoundException; @@ -440,14 +440,14 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali if (this.memberID != null) { s.append("; mbr=").append(this.memberID); } - if ((this.bits & BITS_HAS_PREVIOUS_ID) != 0) { + if (hasPreviousMemberID()) { s.append("; prev=").append(this.previousMemberID); } if (this.distributedSystemId >= 0) { s.append("; ds=").append(this.distributedSystemId); } s.append("; time=").append(getVersionTimeStamp()); - if ((this.bits & BITS_IS_REMOTE_TAG) != 0) { + if (isFromOtherMember()) { s.append("; remote"); } if (this.isAllowedByResolver()) { @@ -544,4 +544,27 @@ public abstract class VersionTag<T extends VersionSource> implements DataSeriali } return true; } + + /** + * Set any bits in the given bitMask on the bits field + */ + private void setBits(int bitMask) { + int oldBits; + int newBits; + do { + oldBits = this.bits; + newBits = oldBits | bitMask; + } while (!bitsUpdater.compareAndSet(this, oldBits, newBits)); + } + /** + * Clear any bits not in the given bitMask from the bits field + */ + private void clearBits(int bitMask) { + int oldBits; + int newBits; + do { + oldBits = this.bits; + newBits = oldBits & bitMask; + } while (!bitsUpdater.compareAndSet(this, oldBits, newBits)); + } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/44289546/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/AbstractVersionTagTestBase.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/AbstractVersionTagTestBase.java b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/AbstractVersionTagTestBase.java new file mode 100644 index 0000000..bf0ce43 --- /dev/null +++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/AbstractVersionTagTestBase.java @@ -0,0 +1,92 @@ +/* + * 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 com.gemstone.gemfire.internal.cache.versions; + +import static org.junit.Assert.*; + +import org.junit.Before; +import org.junit.Test; + +public abstract class AbstractVersionTagTestBase { + @SuppressWarnings("rawtypes") + protected abstract VersionTag createVersionTag(); + + @SuppressWarnings("rawtypes") + private VersionTag vt; + + @Before + public void setup() { + this.vt = createVersionTag(); + } + @Test + public void testFromOtherMemberBit() { + assertEquals(false, vt.isFromOtherMember()); + vt.setIsRemoteForTesting(); + assertEquals(true, vt.isFromOtherMember()); + } + + @Test + public void testTimeStampUpdatedBit() { + assertEquals(false, vt.isTimeStampUpdated()); + vt.setTimeStampApplied(true); + assertEquals(true, vt.isTimeStampUpdated()); + vt.setTimeStampApplied(false); + assertEquals(false, vt.isTimeStampUpdated()); + } + + @Test + public void testGatewayTagBit() { + assertEquals(false, vt.isGatewayTag()); + vt.setIsGatewayTag(true); + assertEquals(true, vt.isGatewayTag()); + vt.setIsGatewayTag(false); + assertEquals(false, vt.isGatewayTag()); + } + + @Test + public void testRecordedBit() { + assertEquals(false, vt.isRecorded()); + vt.setRecorded(); + assertEquals(true, vt.isRecorded()); + } + + @SuppressWarnings("unchecked") + @Test + public void testPreviousMemberIDBit() { + assertEquals(false, vt.hasPreviousMemberID()); + vt.setPreviousMemberID(null); + assertEquals(true, vt.hasPreviousMemberID()); + } + + @Test + public void testPosDupBit() { + assertEquals(false, vt.isPosDup()); + vt.setPosDup(true); + assertEquals(true, vt.isPosDup()); + vt.setPosDup(false); + assertEquals(false, vt.isPosDup()); + } + + @Test + public void testAllowedByResolverBit() { + assertEquals(false, vt.isAllowedByResolver()); + vt.setAllowedByResolver(true); + assertEquals(true, vt.isAllowedByResolver()); + vt.setAllowedByResolver(false); + assertEquals(false, vt.isAllowedByResolver()); + } +} http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/44289546/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/VMVersionTagTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/VMVersionTagTest.java b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/VMVersionTagTest.java new file mode 100644 index 0000000..4e39f3d --- /dev/null +++ b/geode-core/src/test/java/com/gemstone/gemfire/internal/cache/versions/VMVersionTagTest.java @@ -0,0 +1,32 @@ +/* + * 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 com.gemstone.gemfire.internal.cache.versions; + +import org.junit.experimental.categories.Category; + +import com.gemstone.gemfire.test.junit.categories.UnitTest; + +@Category(UnitTest.class) +public class VMVersionTagTest extends AbstractVersionTagTestBase { + + @SuppressWarnings("rawtypes") + @Override + protected VersionTag createVersionTag() { + return new VMVersionTag(); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/44289546/geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedDataSerializables.txt ---------------------------------------------------------------------- diff --git a/geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedDataSerializables.txt b/geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedDataSerializables.txt index fe21fbf..d2204a0 100644 --- a/geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedDataSerializables.txt +++ b/geode-core/src/test/resources/com/gemstone/gemfire/codeAnalysis/sanctionedDataSerializables.txt @@ -1975,8 +1975,8 @@ fromData,212,2a2a2bb600bdb500132bb900be01003d2a1c047e04a0000704a7000403b500972ab toData,239,2ab4001499000dbb00b05912b8b700b2bf2a2ab400132bb600b9033d2ab400979900071c04803d2b1cb900ba02002b2ab4000bb60035b900bb03002b2ab4000cb60035b900bb03002b2ab40025b60033b900ba02002ab40025b60099b9002b01004e2db9002c010099002b2db9002d0100c0002e3a042a1904b9002f0100c0006e2bb600b91904b9003001002bb800bca7ffd22b2ab40026b60033b900ba02002ab40026b60099b9002b01004e2db9002c01009900332db9002d0100c0002e3a042a1904b9002f0100c0006e2bb600b92b1904b900300100c0003eb6003fb900bb0300a7ffca2ab4001f2bb800bcb1 com/gemstone/gemfire/internal/cache/versions/VersionTag,2 -fromData,201,2bb9002001003db20012b20013b900140200990022b20012b20013122105bd001659032ab600175359041cb8001853b9001904002a2bb900200100b500012a2bb900220100b5000d1c077e9900132a2bb900230100121c7eb50003a7000f2a2bb900240100027eb500031c10107e99000d2a2bb900230100b500072a2bb900240100b5000a2a2bb80025b500041c047e99000c2a2a2bb60026b5000b1c057e99001e1c10087e99000e2a2ab4000bb5000ca7000c2a2a2bb60026b5000c2a59b40001101080b50001b1 -toData,269,033e0336042ab400031211a2000a0436041d07803e2ab400079900081d1010803e2ab4000bc6000b1c9900071d04803e2ab4000cc6001b1d05803e2ab4000c2ab4000ba6000c1c9900081d1008803eb20012b20013b900140200990022b20012b20013121505bd001659032ab600175359041db8001853b9001904002b1db9001a02002b2ab40001b9001a02002b2ab4000db9001b020015049900132b2ab40003121c7eb9001a0200a7000d2b2ab40003b9001d02002ab4000799000d2b2ab40007b9001a02002b2ab4000ab9001d02002ab400042bb8001e2ab4000bc600101c99000c2a2ab4000b2bb6001f2ab4000cc6001b2ab4000c2ab4000ba600071c9a000c2a2ab4000c2bb6001fb1 +fromData,197,2bb9002201003db20014b20015b900160200990022b20014b20015122305bd001859032ab600195359041cb8001a53b9001b0400b200242a2bb900220100b600252a2bb900260100b500101c077e9900132a2bb900270100121e7eb50006a7000f2a2bb900280100027eb500061c10107e99000d2a2bb900270100b5000a2a2bb900280100b5000d2a2bb80029b500071c047e99000c2a2a2bb6002ab5000e1c057e99001e1c10087e99000e2a2ab4000eb5000fa7000c2a2a2bb6002ab5000f2ab6002bb1 +toData,269,033e0336042ab400061213a2000a0436041d07803e2ab4000a9900081d1010803e2ab4000ec6000b1c9900071d04803e2ab4000fc6001b1d05803e2ab4000f2ab4000ea6000c1c9900081d1008803eb20014b20015b900160200990022b20014b20015121705bd001859032ab600195359041db8001a53b9001b04002b1db9001c02002b2ab40002b9001c02002b2ab40010b9001d020015049900132b2ab40006121e7eb9001c0200a7000d2b2ab40006b9001f02002ab4000a99000d2b2ab4000ab9001c02002b2ab4000db9001f02002ab400072bb800202ab4000ec600101c99000c2a2ab4000e2bb600212ab4000fc6001b2ab4000f2ab4000ea600071c9a000c2a2ab4000f2bb60021b1 com/gemstone/gemfire/internal/cache/wan/GatewaySenderAdvisor$GatewaySenderProfile,4 fromData,282,2a2bb700082a2bb80009b5000a2a2bb9000b0100b5000c2a2bb9000d0100b5000e2a2bb9000f0100b500102a2bb9000f0100b500112a2bb9000f0100b500122a2bb9000f0100b500132a2bb9000f0100b500142a2bb9000d0100b500152a2bb9000f0100b500162a2bb80017b500042a2bb80017b500052a2bb80017b500062a2bb9000f0100b500182a2bb9000d0100b500192bb8001ab2001bb6001c9c00552bb8001dc0001e4d2cc600412cb6001fb20020b60021b6002299000d2ab20020b50023a7002c2cb6001fb20024b60021b6002299000d2ab20024b50023a700122ab20025b50023a700082a01b50023a7000e2a2bb8001dc00026b500232bb800273d1c9900162abb002859b70029b5002a2ab4002a2bb8002bb1