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

Add threadpool for broadcaster #6488

Conversation

HenrikJannsen
Copy link
Collaborator

Fixes #6480

At broadcast there can be many messages been sent and if the tor connections are slow the sendMessage threadpool can get exhausted. We did not handle the RejectedExecutionException so it escalated as uncaught exception to the user in from of an error popup (pre 1.9.7 it was swallowed in a debug log, so I assume we had RejectedExecutionException aslo earlier).
We catch now RejectedExecutionException at the sendMessage method as well at the uncaughtExceptionHandler to only log an error but not open an error popup.
We add a dedicated threadpool to the broadcaster so that we can separate that pool from the common sendMessage pool. In case a burst of broadcasts would exhaust the broadcaster pool we get dropped some of the broadcast messages which has little impact and risk. The common sendMessage pool will be not affected in such a scenario, thus important messages like trade messages are not at risk.
We reduced also the keepAliveTime to 30 sec to lower resource comsumption.

…error instead calling the uncaughtExceptionHandler

Signed-off-by: HenrikJannsen <[email protected]>
Add try/catch to handle RejectedExecutionException

Signed-off-by: HenrikJannsen <[email protected]>
Signed-off-by: HenrikJannsen <[email protected]>
The broadcasting consumes most of the threads but has lower priority than other messages being sent.
By separating that thread pool from the common sendMessage executor we can reduce the risk that a burst of
broadcasts exhausts the thread pool and might drop send message tasks.

Signed-off-by: HenrikJannsen <[email protected]>
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

utACK

@alejandrogarcia83 alejandrogarcia83 merged commit a8a0c0e into bisq-network:master Dec 29, 2022
@alejandrogarcia83 alejandrogarcia83 added this to the v1.9.9 milestone Dec 29, 2022
@HenrikJannsen HenrikJannsen deleted the add_threadpool_for_broadcaster branch December 29, 2022 17:37
This was referenced Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error Task TrustedListenableFutureTask@7d185bf2[status=PENDING, info=[task=[running=[NOT STARTED YET]
2 participants