Make sure DocumentsWriterPerThread#getAndLock never returns null on a non-empty queue.

[jpountz]

Before this change, DocumentsWriterPerThread#getAndLock could sometimes
return null even though the queue was empty at no point in time. The
practical implication is that we can end up with more DWPTs in memory than
indexing threads, which, while not strictly a bug, may require doing more
merging than we'd like later on.

I ran luceneutil's IndexGeonames with this change, and
DocumentsWriterPerThread#getAndLock was not the main source of
contention.

Closes #12649 #12916

[uschindler]

It looks like this does not fix the issue in #12916. The bug is there but it does not look like the cause for #12916.

[dweiss]

For what it's worth, I've tried reproducing this on Adrien's branch with:

gradlew -p lucene/core -Dtests.seed=F7B4CD7A5624D5EC
beast --tests TestIndexWriterThreadsToSegments.testSegmentCountOnFlushRandom -Dtests.jvmargs="-XX:+UseCompressedOops" -Ptests.iters=1000 -Ptests.dups=100

but nope, no fails.

[uschindler]

I get it to fail with both OpenJ9 and Hotspot on this branch, see #12916

[jpountz]

OK, I think I understand why the test is still failing, this seems to be because we unlock the DWPT after putting it back into the queue. But there is also a good reason to do so. I need to think more about it.

[jpountz]

I have a new iteration of this change, which now also accounts for how DocumentsWriterPerThreadPool may lock DocumentsWriterPerThread instances while they are still in the pool. I'm at 2 hours of beasting with J9 without a failure so far.

[jpountz]

If there are no concerns, I plan on merging this PR soon.

[uschindler]

Hi,
sorry I missed to test this. I started my beasting and try to reproduce it.
Will report back.

[uschindler]

I beasted this PR and it produced on my Ryzen CPU under load no failures. So it looks like it fixed the issue.

The AtomicInteger is only incremented so at some point it overflows. But when analyzing the code, the actual number does not matter, it just needs to be different to retry, correct? So when it overflows it does not do any harm.

[uschindler]

FYI, I did not try to run the beasting for hours because previously it was failing very fast on a heavy loaded Ryzen CPU with both Hotspot and OpenJ9. Now its stable, so I trust the results also after 2 hours (1 hour with Hotspot, 1 hour with OpenJ9).

[jpountz]

Thanks a lot @uschindler !