You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bb...@apache.org on 2023/02/18 00:20:49 UTC
[hbase] branch branch-2.5 updated: HBASE-27648 CopyOnWriteArrayMap does not honor contract of ConcurrentMap.putIfAbsent (#5031)
This is an automated email from the ASF dual-hosted git repository.
bbeaudreault pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.5 by this push:
new e845e2e4dc3 HBASE-27648 CopyOnWriteArrayMap does not honor contract of ConcurrentMap.putIfAbsent (#5031)
e845e2e4dc3 is described below
commit e845e2e4dc3fe5d398c8092f5023ef80c2f0fd01
Author: Bryan Beaudreault <bb...@apache.org>
AuthorDate: Fri Feb 17 19:17:52 2023 -0500
HBASE-27648 CopyOnWriteArrayMap does not honor contract of ConcurrentMap.putIfAbsent (#5031)
Signed-off-by: Duo Zhang <zh...@apache.org>
---
.../apache/hadoop/hbase/types/CopyOnWriteArrayMap.java | 3 ++-
.../apache/hadoop/hbase/types/TestCopyOnWriteMaps.java | 15 ++++++++++++++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/CopyOnWriteArrayMap.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/CopyOnWriteArrayMap.java
index bc184b1098a..1b9ea09f31d 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/CopyOnWriteArrayMap.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/CopyOnWriteArrayMap.java
@@ -332,7 +332,8 @@ public class CopyOnWriteArrayMap<K, V> extends AbstractMap<K, V>
if (index < 0) {
COWEntry<K, V> newEntry = new COWEntry<>(key, value);
this.holder = current.insert(-(index + 1), newEntry);
- return value;
+ // putIfAbsent contract requires returning null if no previous entry exists
+ return null;
}
return current.entries[index].getValue();
}
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestCopyOnWriteMaps.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestCopyOnWriteMaps.java
index 7c8a7d2f4b6..5487997afea 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestCopyOnWriteMaps.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestCopyOnWriteMaps.java
@@ -41,6 +41,7 @@ public class TestCopyOnWriteMaps {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestCopyOnWriteMaps.class);
+ private static final int MAX_INITIAL_ENTRIES = 10_000;
private static final int MAX_RAND = 10 * 1000 * 1000;
private ConcurrentNavigableMap<Long, Long> m;
private ConcurrentSkipListMap<Long, Long> csm;
@@ -50,7 +51,7 @@ public class TestCopyOnWriteMaps {
m = new CopyOnWriteArrayMap<>();
csm = new ConcurrentSkipListMap<>();
- for (long i = 0; i < 10000; i++) {
+ for (long i = 0; i < MAX_INITIAL_ENTRIES; i++) {
long o = ThreadLocalRandom.current().nextLong(MAX_RAND);
m.put(i, o);
csm.put(i, o);
@@ -60,6 +61,18 @@ public class TestCopyOnWriteMaps {
csm.put(0L, o);
}
+ @Test
+ public void testPutIfAbsent() {
+ long key = MAX_INITIAL_ENTRIES * 2;
+ long val = ThreadLocalRandom.current().nextLong(MAX_RAND);
+ // initial call should return null, then should return previous value
+ assertNull(m.putIfAbsent(key, val));
+ assertEquals(val, (long) m.putIfAbsent(key, val * 2));
+ // test same with csm so we ensure similar semantics
+ assertNull(csm.putIfAbsent(key, val));
+ assertEquals(val, (long) csm.putIfAbsent(key, val * 2));
+ }
+
@Test
public void testSize() {
assertEquals("Size should always be equal", m.size(), csm.size());