Use threadpool for subscription checking #465

e-pettersson-ericsson · 2020-06-11T14:13:47Z

Applicable Issues

Previously new threads were created every time we compared an aggregation towards all the existing subscriptions. Since there was no limit to the amount of thread this caused a huge load on Eiffel Intelligence and it was unable to consume events fast enough.

Description of the Change

Add new thread pool for SubscriptionHandler
Setup pool for SubscriptionHandler in SpringAsyncConfig
Update test and info to use new properties
Add new application properties
Set org.mongodb.driver.protocol.command log out to error
Remove use of "local.server.port" from debug printout
Fix of docker-compose correct path to rules
Add config for subscription handler execution pool in tests
Step version to 2.2.1

Alternate Designs

Benefits

By using a thread pool it is possible to configure the amount of threads if it's needed.

Possible Drawbacks

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: @m-linner-ericsson, @e-pettersson-ericsson @SantoshNC68

- Add new thread pool for SubscriptionHandler - Setup pool for SubscriptionHandler in SpringAsyncConfig - Update test and info to use new properties - Add new application properties - Set org.mongodb.driver.protocol.command log out to error - Remove use of "local.server.port" from debug printout

- Fix of docker-compose correct path to rules - Add config for subscription handler execution pool in tests

tobiasake

Looks good,, I really like the prefix names in thread properties names, so the user that configures the EI instance knows what the thread configuration do.
But change so we follow the Spring properties naming standards, regarding capital letters etc.

default values for new properties

e-pettersson-ericsson · 2020-06-11T14:42:28Z

Looks good,, I really like the prefix names in thread properties names, so the user that configures the EI instance knows what the thread configuration do.
But change so we follow the Spring properties naming standards, regarding capital letters etc.

On master we went for dot notation style in the properties instead of camelCase. So I guess the new properties should follow the same style then. Is "subscription.handler.core.pool.size" better?

Add javadoc on some public methods Simplified complex methods

src/main/java/com/ericsson/ei/subscription/SubscriptionHandler.java

before creating a new client - Added condition in MongoHandler to check if there are any mongoclients if not create a new client.

- Add new thread pool for SubscriptionHandler - Setup pool for SubscriptionHandler in SpringAsyncConfig - Update test and info to use new properties - Add new application properties - Set org.mongodb.driver.protocol.command log out to error - Remove use of "local.server.port" from debug printout

- Fix of docker-compose correct path to rules - Add config for subscription handler execution pool in tests

default values for new properties

before creating a new client - Added condition in MongoHandler to check if there are any mongoclients if not create a new client.

…n/eiffel-intelligence into thread_issue

tobiasake · 2020-06-12T10:06:23Z

...altests/java/com/ericsson/ei/threadingAndWaitlistRepeat/ThreadingAndWaitlistRepeatSteps.java

        "threads.corePoolSize= 3",
        "threads.queueCapacity= 1",
        "threads.maxPoolSize= 4",
+        "subscription-handler.threads.corePoolSize= 3",


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 ?

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?

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...

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.

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?

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.

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.

I am mean, why implement something to fix a issue without testing that specific situation?

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.

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.

Christoffer-Cortes

Looks good, can't find any obvious errors and tests are passing.

m-linner-ericsson added 2 commits June 11, 2020 14:23

Various fixes

b1d43fe

- Fix of docker-compose correct path to rules - Add config for subscription handler execution pool in tests

e-pettersson-ericsson requested a review from a team June 11, 2020 14:17

tobiasake requested changes Jun 11, 2020

View reviewed changes

Revert name change on existing properties and use

887db2e

default values for new properties

Christoffer-Cortes and others added 2 commits June 12, 2020 08:36

2.x Use waitlist queue name as routing key (eiffel-community#461)

1ca3cbf

Remove duplicate logging (eiffel-community#466)

6d1d5dd

Add javadoc on some public methods Simplified complex methods

e-pettersson-ericsson commented Jun 12, 2020

View reviewed changes

src/main/java/com/ericsson/ei/subscription/SubscriptionHandler.java Outdated Show resolved Hide resolved

SantoshNC68 and others added 5 commits June 12, 2020 13:25

Added check to see if the mongo client is null

ead4bfb

before creating a new client - Added condition in MongoHandler to check if there are any mongoclients if not create a new client.

2.x Remove port logging in event handler (eiffel-community#464)

01a661f

Added more checks on mongoclient

b060456

For triggering travis CI

344f465

Trying to trigger travis

ff600ed

e-pettersson-ericsson marked this pull request as ready for review June 12, 2020 08:48

m-linner-ericsson and others added 13 commits June 12, 2020 14:23

Various fixes

0864651

- Fix of docker-compose correct path to rules - Add config for subscription handler execution pool in tests

Revert name change on existing properties and use

0c6fd2f

default values for new properties

Added check to see if the mongo client is null

956e8e5

before creating a new client - Added condition in MongoHandler to check if there are any mongoclients if not create a new client.

Added more checks on mongoclient

2d4892d

For triggering travis CI

950ed52

Trying to trigger travis

613f041

Rebased the changes with 2.0.0-maintainance branch

fe4b684

Merge branch 'threading_fixes' of https://github.com/m-linner-ericsso…

6a63c6a

…n/eiffel-intelligence into thread_issue

rebased with 2.0.0-maintainance branch

ce102df

Revert new connection to mongo in each thread

54740cd

Revert changes in mongodb handler

051a170

Step patch version to 2.0.1

9e67b9a

e-pettersson-ericsson requested review from a team and tobiasake June 12, 2020 09:41

tobiasake reviewed Jun 12, 2020

View reviewed changes

Update default values to match eventhandler threadpool

871dd0d

Christoffer-Cortes approved these changes Jun 12, 2020

View reviewed changes

e-pettersson-ericsson merged commit 5435c7b into eiffel-community:2.0.0-maintenance Jun 12, 2020

m-linner-ericsson mentioned this pull request Sep 15, 2020

Change Event and Aggreagations threads to use only one threadpool #475

Merged

Use threadpool for subscription checking #465

Use threadpool for subscription checking #465

Uh oh!

Conversation

e-pettersson-ericsson commented Jun 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Applicable Issues

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Sign-off

Uh oh!

tobiasake left a comment

Choose a reason for hiding this comment

Uh oh!

e-pettersson-ericsson commented Jun 11, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasake Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasake Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Christoffer-Cortes left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

e-pettersson-ericsson commented Jun 11, 2020 •

edited

Loading

tobiasake Jun 12, 2020 •

edited

Loading

tobiasake Jun 12, 2020 •

edited

Loading