Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock #4066

Merged
merged 11 commits into from
Apr 28, 2024
Merged

[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock #4066

merged 11 commits into from
Apr 28, 2024

Conversation

thetumbled
Copy link
Member

@thetumbled thetumbled commented Sep 1, 2023 •
edited
Loading

Motivation

This is the bug fix simillar to Pulsar. refer to: apache/pulsar#18390.

Changes

Fix bugs in:

ConcurrentLongHashMap
ConcurrentLongHashSet
ConcurrentLongLongHashMap
ConcurrentLongLongPairHashMap
ConcurrentOpenHashMap
ConcurrentOpenHashSet

@thetumbled
Copy link
Member Author

thetumbled commented Sep 18, 2023 •
edited
Loading

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

@zymap zymap added the type/bug label Sep 19, 2023
@zymap zymap added this to the 4.17.0 milestone Sep 19, 2023
@thetumbled
Copy link
Member Author

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.

@eolivelli eolivelli modified the milestones: 4.17.0, 4.18.0 Mar 29, 2024
@thetumbled thetumbled mentioned this pull request Apr 27, 2024
Closed
@thetumbled
Copy link
Member Author

I find that the same concurrency problem is reported in bookkeeper #4316, it is time to merge this fix into the bookkeeper.
PTAL, thanks. @lhotari @hangc0276 @eolivelli @wenbingshen @zymap @shoothzj @horizonzy

@thetumbled thetumbled mentioned this pull request Apr 27, 2024
Closed
@lhotari lhotari mentioned this pull request Apr 27, 2024
Closed
dlg99
dlg99 approved these changes Apr 28, 2024
View reviewed changes
Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Thank you!

@dlg99 dlg99 changed the title Fix ArrayIndexOut0fBoundsException caused by optimistic lock [fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock Apr 28, 2024
@dlg99
Copy link
Contributor

dlg99 commented Apr 28, 2024

retest this please

@eolivelli
Copy link
Contributor

closing/reopening to trigger CI

@eolivelli eolivelli closed this Apr 28, 2024
@eolivelli eolivelli reopened this Apr 28, 2024
eolivelli
eolivelli approved these changes Apr 28, 2024
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

thanks

@dlg99
Copy link
Contributor

dlg99 commented Apr 28, 2024

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'

@thetumbled
Copy link
Member Author

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.

@dlg99 dlg99 merged commit e4faf25 into apache:master Apr 28, 2024
20 checks passed
dlg99 pushed a commit to datastax/bookkeeper that referenced this pull request Apr 29, 2024
@thetumbled @dlg99
[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock (a…
9763323 
…pache#4066)

(cherry picked from commit e4faf25)
@dlg99 dlg99 mentioned this pull request Apr 29, 2024
Merged
@lhotari
Copy link
Member

lhotari commented Apr 30, 2024

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

dlg99 added a commit to datastax/bookkeeper that referenced this pull request May 1, 2024
@dlg99 @thetumbled
[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock (a…
796523d 
…pache#4066) (#18)

(cherry picked from commit e4faf25)

Co-authored-by: thetumbled <[email protected]>
@shoothzj
Copy link
Member

shoothzj commented May 5, 2024

@lhotari done

shoothzj pushed a commit that referenced this pull request May 5, 2024
@thetumbled @shoothzj
[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock (#…
a2dc07c 
…4066)
shoothzj pushed a commit that referenced this pull request May 5, 2024
@thetumbled @shoothzj
[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock (#…
f0ec6b2 
…4066)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
@thetumbled
[fix] Fix ArrayIndexOut0fBoundsException caused by optimistic lock (a…
f10550c 
…pache#4066)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shoothzj shoothzj shoothzj approved these changes

@eolivelli eolivelli eolivelli approved these changes

@dlg99 dlg99 dlg99 approved these changes

@merlimat merlimat Awaiting requested review from merlimat

@hangc0276 hangc0276 Awaiting requested review from hangc0276

@wenbingshen wenbingshen Awaiting requested review from wenbingshen

Assignees

@thetumbled thetumbled

Labels
cherry-picked/branch-4.16 cherry-picked/branch-4.17 release/4.16.6 release/4.17.1 type/bug
Projects
None yet
Milestone
4.18.0
Development

Successfully merging this pull request may close these issues.
6 participants
@thetumbled @dlg99 @eolivelli @lhotari @shoothzj @zymap