ThreadAffinityDocumentsWriterThreadPool checks hasQueuedThreads rather than isLocked [LUCENE-5359]
- Dominant language
- Java
- Stars
- 3.6k
- Forks
- 1.4k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 88
Description
ThreadAffinityDocumentsWriterThreadPool.getAndLock() calls minContendedThreadState() to get a ThreadState, then checks hasQueuedThreads() to determine whether to call newThreadState() or lock the returned ThreadState. It seems to me that it should be checking isLocked() instead; it shouldn't accept an already-locked ThreadState just because there are no other threads queued on it. (Alternately, maybe minContendedThreadState() should prefer to return an unlocked ThreadState, rather than just checking the queue length.)
Empirically, on 4.4.0, when writing an index from 8 threads on an 8-core machine, with the hasQueuedThreads() call, the CPU is usually not saturated, and the writing threads spend up to 15% of their time blocked on the minThreadState.lock() call in getAndLock(); tracing shows that the return value of minContendedThreadState() is often used when it's locked but has no queued threads, even though there are <8 active ThreadStates and newThreadState() would return a fresh one. When changing hasQueuedThreads() to isLocked(), the CPU is usually saturated and getAndLock() doesn't block.
---
Migrated from [LUCENE-5359](https://issues.apache.org/jira/browse/LUCENE-5359) by Nick Tarleton
Contributor guide
Research direction
Start in ThreadAffinityDocumentsWriterThreadPool.getAndLock(), then read minContendedThreadState(), hasQueuedThreads(), isLocked(), and newThreadState() to understand the state-selection path. Verify the reported contention scenario and confirm that an already-locked ThreadState with no queued threads is handled without the avoidable blocking described in the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- search
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 38/100