Fix static DictionaryIndexConfig.DEFAULT_OFFHEAP being actually onheap #10632
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
Codecov Report
@@ Coverage Diff @@
## master #10632 +/- ##
============================================
- Coverage 27.79% 27.74% -0.05%
Complexity 58 58
============================================
Files 2090 2090
Lines 112648 112647 -1
Branches 16988 16988
============================================
- Hits 31307 31255 -52
- Misses 78232 78268 +36
- Partials 3109 3124 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 53 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Jackie-Jiang
approved these changes
Apr 18, 2023
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.
This is a bugfix. There were two situations where Dictionary reader was loaded offheap before index-spi was merged but loaded onheap after that. All the cases were related to other indexes requiring to have a DictionaryReader to be created, so the issue shouldn't affect queries. The original issue was detected by @jadami10 in #10554 (comment).
The reason was the static final instance
DictionaryIndexConfig.DEFAULT_OFFHEAPthat was actually configured to load the index onheap. The normalDictionaryIndexConfig.DEFAULTis already offheap, so there is no need to have the incorrect static final.