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.

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

fix: concurrent modification of processing receievd messages #1807

Merged

Conversation

maitrimangal
Copy link
Member

@maitrimangal maitrimangal commented Oct 31, 2023

When a lot of messages are published, then there is a concurrent modification issue in subscriber callbacks for exactly once. This PR makes the outstandingReciepts a concurrentHashMap, and thus allowing us to remove the synchronized keyword from notifyAckSuccess, and fail because all other data structures in this function are concurrent.
I have tested this as described in the following issue, and am unable to see the modification errors.
Fixes #1778

@maitrimangal maitrimangal requested a review from a team as a code owner October 31, 2023 17:54
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: pubsub Issues related to the googleapis/java-pubsub API. labels Oct 31, 2023
@maitrimangal maitrimangal added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Oct 31, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 31, 2023
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 31, 2023
@kamalaboulhosn
Copy link
Contributor

I expect the synchronized methods are going to make parallel processing much slower and so we should probably remove that and replace it with a concurrent data structure so move the concurrency control to a lower level. It looks like the pendingAcks, pendingNacks, and pendingReceipts do this. We can hopefully then get rid of the synchronized keyword on notifyAckSuccess and notifyAckFailed.

@maitrimangal maitrimangal added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 31, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 31, 2023
@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner October 31, 2023 20:57
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Oct 31, 2023
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. and removed size: s Pull request size is small. labels Nov 2, 2023
@maitrimangal maitrimangal added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 2, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/java-pubsub API. size: xs Pull request size is extra small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ConcurrentModificationException in subscriber callbacks when using exactly-once-delivery
3 participants