Store index metadata file for Lucene text indexes #13948
Conversation
itschrispeck
added
enhancement
release-notes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13948 +/- ##
============================================
+ Coverage 61.75% 63.83% +2.08%
- Complexity 207 1530 +1323
============================================
Files 2436 2621 +185
Lines 133233 144069 +10836
Branches 20636 22034 +1398
============================================
+ Hits 82274 91971 +9697
- Misses 44911 45318 +407
- Partials 6048 6780 +732
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
thanks @itschrispeck for the contribution and detailed information! |
This diff would be a pre-req to do so, but that functionality would still have to be added in I have not added the functionality in this PR since internally we take advantage of the fact that it is not currently possible. When that functionality is added, we'll add some config to enable/disable it as well. |
| CommonsConfigurationUtils.saveToFile(properties, propertiesFile); | ||
| } | ||
|
|
||
| public static TextIndexConfig getUpdatedConfigFromPropertiesFile(File file, TextIndexConfig config) |
There was a problem hiding this comment.
Add javadoc to public methods.
| } | ||
|
|
||
| return (Constructor<QueryParserBase>) queryParserClass.getConstructor(String.class, Analyzer.class); | ||
| } | ||
|
|
||
| public static void writeConfigToPropertiesFile(File indexDir, TextIndexConfig config) { |
There was a problem hiding this comment.
Add javadoc to public methods.
| @@ -300,6 +300,12 @@ public static Constructor<QueryParserBase> getQueryParserWithStringAndAnalyzerTy | |||
| return (Constructor<QueryParserBase>) queryParserClass.getConstructor(String.class, Analyzer.class); | |||
| } | |||
|
|
|||
| /** | |||
| * Writes the config to the properties file. | |||
There was a problem hiding this comment.
Can we be more specific on the fields which are written to the properties files?
|
@chenboat , @itschrispeck - We ran into an NPE with this change. When we don't have all fields populated in the TextIndexConfig, we will run into NPE, via code path that calls. The call path is from newly added code getUpdatedConfigFromPropertiesFile . public AbstractBuilder(TextIndexConfig other) { java.lang.RuntimeException: java.lang.NullPointerException: Cannot invoke "java.util.Collection.toArray()" because "c" is null |
|
Raised a fix #14616 |
Currently the Lucene text index is written using some Analyzer and queried with some QueryParser. Updating these in the table config can cause unpredictable behavior as when the segments are loaded they'll use the new configs but segments were built with the old configs. This can cause incorrect results when querying.
For example,
Analyzer1andQueryPaserForAnalyzer1Analyzer2andQueryParserForAnalyzer2Analyzer2andQueryParserForAnalyzer2QueryParserForAnalyzer2but index usesAnalyzer1Storing metadata per segment allows for compatible QueryParser/Analyzer pairs to be maintained. Now,
4becomes4. segment 1 loaded, Reader uses
QueryParserForAnalyzer1and index usesAnalyzer1.This allows for seamless updates of realtime tables without breaking queries against old segments (provided the
QueryParserandAnalyzerclasses are managed correctly). In the future this metadata can be leveraged by TextIndexHandler to provide better index reload/rebuilding capabilities for Lucene text index.Testing: we've been using this internally for around half a year
tags:
enhancement,docs