Improve job pool configurations #1746
Merged
Conversation
This file contains 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
osoriano
reviewed
Nov 12, 2024
deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/ConfigHelper.java
Show resolved
Hide resolved
osoriano
approved these changes
Nov 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Due to misconfiguration, the job pool used in Teletraan has only one thread. Similar issue fixed in https://github.com/pinternal/rodimus/pull/110
ExecutorServiceused for thread pool has only 1 thread. Changed the work queue type to fix. The configuration is the same as theregularpool in the Rodimus.ExecutorServiceinstance is not managed, so it's not properly shutdown. Use Dropwizard for lifecycle management.Considerations
I didn't have the whole change in Rodimus ported because it was not needed. There are only 3 types of tasks involved in Teletraan, ChangeFeed, NotifyJob and Webhook. They will work fine with just the regular pool configuration.
Test plan
This change is a subset of changes we made to Rodimus and it's been working well. For sanity tests, I deployed a build with this change and verified that change feed events are published and metrics are emitted.