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
LUCENE-9074: Slice Allocation Circuit Breakers in IndexSearcher #1049
Conversation
This commit introduces accounting for the queue length of the ExecutorService being used to perform concurrent search when allocating slices for an IndexSearcher. This commit also introduces an abstraction to define custom parameters for sealing bulkheads under heavy node stress to allow better predictable behaviour of latencies under varying stress
c2dbb9c
to
c0a80f2
Compare
lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
Outdated
Show resolved
Hide resolved
lucene/core/src/java/org/apache/lucene/search/QueueSizeBasedCircuitBreaker.java
Outdated
Show resolved
Hide resolved
lucene/core/src/java/org/apache/lucene/search/SliceAllocationCircuitBreaker.java
Outdated
Show resolved
Hide resolved
lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
Outdated
Show resolved
Hide resolved
lucene/core/src/java/org/apache/lucene/search/QueueSizeBasedCircuitBreaker.java
Outdated
Show resolved
Hide resolved
Raised another iteration -- please see and let me know your comments |
c5fc00d
to
44edde3
Compare
* Return true if the circuit breaker condition has triggered, | ||
* false otherwise | ||
*/ | ||
boolean hasCircuitBreakerTriggered(); |
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.
Maybe we should move the logic of assigning tasks to threads to this class to give it more flexibility instead of just exposing whether the pool is running over capacity. I'm thinking of something that could look like void invokeAll(Collection<Runnable> tasks)
(similar to ForkJoinPool), which could then decide to merge some runnables together, run some of them on the current thread, etc.
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.
Would that also include creating the actual slices? We would need this class to be involved in that process as well since it can control the number of slices being created. Or maybe we create the slices as we do today, but can run multiple slices on caller thread when things are hot?
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 was thinking the latter indeed, having more and more slices in a single task as the number of entries in the queue increases.
Superseded by #1214 |
This commit introduces accounting for the queue length of the ExecutorService
being used to perform concurrent search when allocating slices for an
IndexSearcher. This commit also introduces an abstraction to define
custom parameters for sealing bulkheads under heavy node stress
to allow better predictable behaviour of latencies under varying stress