Reduce contention in DocumentsWriterFlushControl. #12198
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
lucene-util's `IndexGeoNames` benchmark is heavily contended when running with many indexing threads, 20 in my case. The main offender is `DocumentsWriterFlushControl#doAfterDocument`, which runs after every index operation to update doc and RAM accounting. This change reduces contention by only updating RAM accounting if the amount of RAM consumption that has not been committed yet by a single DWPT is at least 0.1% of the total RAM buffer size. This effectively batches updates to RAM accounting, similarly to what happens when using `IndexWriter#addDocuments` to index multiple documents at once. Since updates to RAM accounting may be batched, `FlushPolicy` can no longer distinguish between inserts, updates and deletes, so all 3 methods got merged into a single one. With this change, `IndexGeoNames` goes from ~22s to ~19s and the main offender for contention is now `DocumentsWriterPerThreadPool#getAndLock`.
jpountz
added a commit
to jpountz/lucene
that referenced
this pull request
Mar 10, 2023
Obtaining a DWPT and putting it back into the pool is subject to contention. This change reduces contention by using 8 sub pools that are tried sequentially. When applied on top of apache#12198, this reduces the time to index geonames with 20 threads from ~19s to ~16-17s.
jpountz
added a commit
to jpountz/lucene
that referenced
this pull request
Mar 10, 2023
Obtaining a DWPT and putting it back into the pool is subject to contention. This change reduces contention by using 8 sub pools that are tried sequentially. When applied on top of apache#12198, this reduces the time to index geonames with 20 threads from ~19s to ~16-17s.
…in absolute value.
s1monw
approved these changes
Mar 15, 2023
lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushControl.java
Outdated
Show resolved
Hide resolved
| return indexWriterConfig.getMaxBufferedDocs() != IndexWriterConfig.DISABLE_AUTO_FLUSH; | ||
| } | ||
|
|
||
| @Override | ||
| public long flushOnRAMGranularity() { |
There was a problem hiding this comment.
I don't think this needs to be an abstract method. I am not sure anyone users this interface at all except of tests and unless we override this method in a test I think it can be an impl detail of the only caller.
…ushControl.java Co-authored-by: Simon Willnauer <simonw@apache.org>
jpountz
added a commit
that referenced
this pull request
Mar 15, 2023
lucene-util's `IndexGeoNames` benchmark is heavily contended when running with many indexing threads, 20 in my case. The main offender is `DocumentsWriterFlushControl#doAfterDocument`, which runs after every index operation to update doc and RAM accounting. This change reduces contention by only updating RAM accounting if the amount of RAM consumption that has not been committed yet by a single DWPT is at least 0.1% of the total RAM buffer size. This effectively batches updates to RAM accounting, similarly to what happens when using `IndexWriter#addDocuments` to index multiple documents at once. Since updates to RAM accounting may be batched, `FlushPolicy` can no longer distinguish between inserts, updates and deletes, so all 3 methods got merged into a single one. With this change, `IndexGeoNames` goes from ~22s to ~19s and the main offender for contention is now `DocumentsWriterPerThreadPool#getAndLock`. Co-authored-by: Simon Willnauer <simonw@apache.org>
jpountz
added a commit
that referenced
this pull request
Mar 15, 2023
Obtaining a DWPT and putting it back into the pool is subject to contention. This change reduces contention by using 8 sub pools that are tried sequentially. When applied on top of #12198, this reduces the time to index geonames with 20 threads from ~19s to ~16-17s.
jpountz
added a commit
that referenced
this pull request
Mar 15, 2023
Obtaining a DWPT and putting it back into the pool is subject to contention. This change reduces contention by using 8 sub pools that are tried sequentially. When applied on top of #12198, this reduces the time to index geonames with 20 threads from ~19s to ~16-17s.
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.
lucene-util's
IndexGeoNamesbenchmark is heavily contended when running with many indexing threads, 20 in my case. The main offender isDocumentsWriterFlushControl#doAfterDocument, which runs after every index operation to update doc and RAM accounting.This change reduces contention by only updating RAM accounting if the amount of RAM consumption that has not been committed yet by a single DWPT is at least 0.1% of the total RAM buffer size or 16kB. This effectively batches updates to RAM accounting, similarly to what happens when using
IndexWriter#addDocumentsto index multiple documents at once. Since updates to RAM accounting may be batched,FlushPolicycan no longer distinguish between inserts, updates and deletes, so all 3 methods got merged into a single one.With this change,
IndexGeoNamesgoes from ~22s to ~19s and the main offender for contention is nowDocumentsWriterPerThreadPool#getAndLock.