-
Notifications
You must be signed in to change notification settings - Fork 73
Use threadpool for subscription checking #465
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
Merged
e-pettersson-ericsson
merged 24 commits into
eiffel-community:2.0.0-maintenance
from
m-linner-ericsson:threading_fixes
Jun 12, 2020
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
e5ed33b
Limit SubcriptionHandler threading
m-linner-ericsson b1d43fe
Various fixes
m-linner-ericsson 887db2e
Revert name change on existing properties and use
e-pettersson-ericsson 1ca3cbf
2.x Use waitlist queue name as routing key (#461)
Christoffer-Cortes 6d1d5dd
Remove duplicate logging (#466)
e-pettersson-ericsson ead4bfb
Added check to see if the mongo client is null
01a661f
2.x Remove port logging in event handler (#464)
Christoffer-Cortes b060456
Added more checks on mongoclient
344f465
For triggering travis CI
ff600ed
Trying to trigger travis
3004823
Limit SubcriptionHandler threading
m-linner-ericsson 0864651
Various fixes
m-linner-ericsson 0c6fd2f
Revert name change on existing properties and use
e-pettersson-ericsson 956e8e5
Added check to see if the mongo client is null
2d4892d
Added more checks on mongoclient
950ed52
For triggering travis CI
613f041
Trying to trigger travis
fe4b684
Rebased the changes with 2.0.0-maintainance branch
6a63c6a
Merge branch 'threading_fixes' of https://github.com/m-linner-ericsso…
ce102df
rebased with 2.0.0-maintainance branch
54740cd
Revert new connection to mongo in each thread
e-pettersson-ericsson 051a170
Revert changes in mongodb handler
e-pettersson-ericsson 9e67b9a
Step patch version to 2.0.1
e-pettersson-ericsson 871dd0d
Update default values to match eventhandler threadpool
e-pettersson-ericsson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this not be a bottleneck if EventHandler thread works faster than the SubscriptionHandler,, we will queue up more and more threads. So Subscriptionhandler threadpool should be much bigger than Eventhandler threadpool.
This could be a good load test scenario, that tests number SubscritionHandler threads needed for different EventHandler threadpool sizes.
What happens if threads.queueCapacity is reached, is those new threads never created and Aggregations never Subscription checked and no triggering is happening ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm not sure if it will be a bottleneck (or if it's a big risk) . The problem we saw before was that an infinite amount of threads were created from SubscriptionHandler and because it reads from the database, it blocked all other processing of events (all EventHandler threads were blocked). Now at least we have a limit on the amount of threads allowed for subscription handler. It's a trade off, which one do we want to prioritize: processing events from rabbitmq or trigger subscriptions as fast as possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to do a "baton pass" so that you wait to process until a thread is available? But then I guess both operations they might as well be in the same thread process...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explained the situation in a chat.
Short summary of the situation I am trying to explain:
If EventHandler thread (event consumer) executes much faster than SubscriptionHandler that executes much slower due to MongoDb connection,queries, updates and read read of big JSON objects, then a lot of threads will be queuing up in SubscriptionHandler thread pool.
And what happens when that queue is full.
If RabbitMq queue has 100 000 events, than we almost must have 100 000 in queue capacity in SubscriptionHandler treadpool queue, to be on safe side so we don't loose any aggregation subscription checks that is made by SubscriptionHandler thread.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If eventHandler threads blocks until a new thread slot is free in SubscriptionHandler thread queue before creating the thread in queue, then it will not be a problem, I think.
Someone can maybe check that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your point @tobiasake but it's hard to test such a scenario. For now I have the same amount of threads in both pools. And don't forget that the event handler also does read/write to mongodb to merge and update aggregations so I don't think it executes so much faster than subscription handler threads do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it would use bigger event linked chains that causes bigger Aggregations and casuign longer/bigger opeations agains MongoDb
It also possible to mock som functions in code and add sleeps in subscriptionHandler threads,, so it takes longer time to process subscriptionhandler threads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am mean, why implement something to fix a issue without testing that specific situation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds complex, doesn't the thread pool have some sort of queue? This queue would keep growing though if the SubscriptionHandler is slower and more and more events are coming in.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of what I can understand on the info about ThreadPool,, queue throws exeception if queue is full. So Aggregation and matching subscription will not happen for those aggreagation that happens when subscriptionQueueHandler Queue is full. So alot of Subscripion triggering will never happen if its alot of events in MessageBus queue and EI consumes more event than number of SubscritpionHandler threads can handle due to the ThreadPool limitations.