bookkeeper icon indicating copy to clipboard operation
bookkeeper copied to clipboard

Fix ArrayIndexOut0fBoundsException caused by optimistic lock

Open thetumbled opened this issue 2 years ago • 2 comments

Motivation

This is the bug fix simillar to Pulsar. refer to: https://github.com/apache/pulsar/pull/18390.

Changes

Fix bugs in:

ConcurrentLongHashMap
ConcurrentLongHashSet
ConcurrentLongLongHashMap
ConcurrentLongLongPairHashMap
ConcurrentOpenHashMap
ConcurrentOpenHashSet

thetumbled avatar Sep 01 '23 06:09 thetumbled

Could you help to reviw this PR? @eolivelli @sijie @hangc0276 @zymap Thanks.

thetumbled avatar Sep 18 '23 07:09 thetumbled

Could you help to reviw this PR? @eolivelli @dlg99 @hangc0276 @nicoloboschi @shoothzj @zymap @wenbingshen This bug could happen when the concurrent data structure shrink it's capacity.

thetumbled avatar Dec 08 '23 06:12 thetumbled

I find that the same concurrency problem is reported in bookkeeper https://github.com/apache/bookkeeper/issues/4316, it is time to merge this fix into the bookkeeper. PTAL, thanks. @lhotari @hangc0276 @eolivelli @wenbingshen @zymap @shoothzj @horizonzy

thetumbled avatar Apr 27 '24 07:04 thetumbled

retest this please

dlg99 avatar Apr 28 '24 01:04 dlg99

closing/reopening to trigger CI

eolivelli avatar Apr 28 '24 02:04 eolivelli

Please fix the checkstyle


[INFO] There are 6 errors reported by Checkstyle 9.3 with buildtools/src/main/resources/bookkeeper/checkstyle.xml ruleset.
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongHashMapTest.java:[224,29] (whitespace) WhitespaceAfter: ',' is not followed by whitespace.
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongLongPairHashMapTest.java:[41,1] (imports) ImportOrder: Extra separation in import group before 'org.apache.bookkeeper.util.collections.ConcurrentLongLongPairHashMap.LongPair'
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongHashSetTest.java:[40,1] (imports) ImportOrder: Extra separation in import group before 'org.junit.Test'
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongLongHashMapTest.java:[42,1] (imports) ImportOrder: Extra separation in import group before 'org.apache.bookkeeper.util.collections.ConcurrentLongLongHashMap.LongLongFunction'
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentOpenHashMapTest.java:[220,32] (whitespace) WhitespaceAfter: ',' is not followed by whitespace.
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentOpenHashSetTest.java:[39,1] (imports) ImportOrder: Extra separation in import group before 'org.junit.Test'

dlg99 avatar Apr 28 '24 02:04 dlg99

Please fix the checkstyle


[INFO] There are 6 errors reported by Checkstyle 9.3 with buildtools/src/main/resources/bookkeeper/checkstyle.xml ruleset.
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongHashMapTest.java:[224,29] (whitespace) WhitespaceAfter: ',' is not followed by whitespace.
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongLongPairHashMapTest.java:[41,1] (imports) ImportOrder: Extra separation in import group before 'org.apache.bookkeeper.util.collections.ConcurrentLongLongPairHashMap.LongPair'
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongHashSetTest.java:[40,1] (imports) ImportOrder: Extra separation in import group before 'org.junit.Test'
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentLongLongHashMapTest.java:[42,1] (imports) ImportOrder: Extra separation in import group before 'org.apache.bookkeeper.util.collections.ConcurrentLongLongHashMap.LongLongFunction'
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentOpenHashMapTest.java:[220,32] (whitespace) WhitespaceAfter: ',' is not followed by whitespace.
Error:  src/test/java/org/apache/bookkeeper/util/collections/ConcurrentOpenHashSetTest.java:[39,1] (imports) ImportOrder: Extra separation in import group before 'org.junit.Test'

fixed, please help to re-trigger the CI, thanks.

thetumbled avatar Apr 28 '24 02:04 thetumbled

@dlg99 @eolivelli Please cherry-pick this PR to branch-4.16.

lhotari avatar Apr 30 '24 14:04 lhotari

@lhotari done

hezhangjian avatar May 05 '24 06:05 hezhangjian

java.lang.ArrayIndexOutOfBoundsException: 1774 at org.apache.bookkeeper.util.collections.ConcurrentLongHashMap$Section.get(ConcurrentLongHashMap.java:346) ~[bookkeeper-server-4.14_vivo-1.5.3-20251114.065856-16.jar:4.14_vivo-1.5.3-SNAPSHOT] at org.apache.bookkeeper.util.collections.ConcurrentLongHashMap.get(ConcurrentLongHashMap.java:206) ~[bookkeeper-server-4.14_vivo-1.5.3-20251114.065856-16.jar:4.14_vivo-1.5.3-SNAPSHOT]

@thetumbled I cherry-pick this PR, but the error still occurred, although the probability of it appearing was greatly reduced. Should we catch the exception and perform lock upgrade when accessing long storedKey = keys[bucket] out of bounds?

keyboardbobo avatar Nov 17 '25 08:11 keyboardbobo

java.lang.ArrayIndexOutOfBoundsException: 1774 at org.apache.bookkeeper.util.collections.ConcurrentLongHashMap$Section.get(ConcurrentLongHashMap.java:346) ~[bookkeeper-server-4.14_vivo-1.5.3-20251114.065856-16.jar:4.14_vivo-1.5.3-SNAPSHOT] at org.apache.bookkeeper.util.collections.ConcurrentLongHashMap.get(ConcurrentLongHashMap.java:206) ~[bookkeeper-server-4.14_vivo-1.5.3-20251114.065856-16.jar:4.14_vivo-1.5.3-SNAPSHOT]

@thetumbled I cherry-pick this PR, but the error still occurred, although the probability of it appearing was greatly reduced. Should we catch the exception and perform lock upgrade when accessing long storedKey = keys[bucket] out of bounds?

@keyboardbobo please report a new issue and share your stack trace.

lhotari avatar Nov 17 '25 08:11 lhotari