OAK-12054: Refactor creation of ThreadPoolExecutors#2679
OAK-12054: Refactor creation of ThreadPoolExecutors#2679thomasmueller merged 1 commit intoapache:trunkfrom
Conversation
|
@bhabegger good catch, thanks for the PR; can you create a dedicated OAK ticket for it? |
4318b53 to
e61cba0
Compare
Well, in fact it was hinted to me by @thomasmueller as a pending task that I took for my onboarding :) So, I don't have much credit for catching this;) |
e61cba0 to
33271ab
Compare
33271ab to
eb16e23
Compare
eb16e23 to
28c2927
Compare
thomasmueller
left a comment
There was a problem hiding this comment.
There is a risk if we change the current behavior, even if the current behavior is not what was intended originally: we could have out-of-memory, concurrency issues etc.
So either the PR should not change the current behavior, or at least, we should have a way to switch back to the old behavior, in case of issues. E.g. with a feature toggle.
Given that we don't currently see issue with the current behavior (other than it's not intended, and can confuse people, etc.), I think we should keep the current behavior.
687a16b to
443d867
Compare
5f19daa to
60ee9fa
Compare
60ee9fa to
3f44d08
Compare
…e in fact only running one thread)
3f44d08 to
2a6f894
Compare
| } | ||
|
|
||
| public static final String REPOSITORY_HOME = "repository.home"; | ||
| private static final int INDEX_COPIER_POOL_SIZE = 5; |
There was a problem hiding this comment.
Nit: if we only use it in one place, it seems not useful to have a constant.
There was a problem hiding this comment.
I see 2 benefits though:
- readability by having a name rather than just a number
- having the "setup" of the class at the top (these are potential candidate parameters)
There was a problem hiding this comment.
(But I won't fight over it :))
…e in fact only running one thread) (#2679)
…e in fact only running one thread) (#2679)
Some instantiations of ThreadPoolExecutor were setting a different corePoolSize and maxPoolSize even though they were using an unbounded LinkedBlockingQueue. The pattern was repeated multiple times. This PR attempts to fix and mutualize the ThreadPoolExecutor creation code.