You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "abhioncbr (via GitHub)" <gi...@apache.org> on 2024/04/10 09:53:36 UTC
[PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
abhioncbr opened a new pull request, #12866:
URL: https://github.com/apache/pinot/pull/12866
As per the [issue](https://github.com/apache/pinot/issues/12860), This PR upgrades the Lucene dependency to 9.10.0.
Several code changes have to be made to make it compatible with a newer version of Lucene.
With this PR, we can close the other similar [PR](https://github.com/apache/pinot/pull/12844). Thanks
cc: @Jackie-Jiang
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #12866:
URL: https://github.com/apache/pinot/pull/12866#issuecomment-2071043920
I don't fully follow why did it break. Following the same logic in #11857 should work. Basically when downgrading and Pinot doesn't understand the new format, it should try to generate the old format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559807956
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/vector/lucene95/HnswVectorsFormat.java:
##########
@@ -19,10 +19,10 @@
package org.apache.pinot.segment.local.segment.creator.impl.vector.lucene95;
import java.io.IOException;
+import org.apache.lucene.backward_codecs.lucene95.Lucene95HnswVectorsFormat;
Review Comment:
Does this mean after upgrading Lucece to 0.9.10:
1. We can only write index with lucene99 based codecs?
2. If a new generated index is served by an old server (running Lucene 9.8.0), will it break?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559767788
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/vector/lucene95/HnswVectorsFormat.java:
##########
@@ -19,10 +19,10 @@
package org.apache.pinot.segment.local.segment.creator.impl.vector.lucene95;
import java.io.IOException;
+import org.apache.lucene.backward_codecs.lucene95.Lucene95HnswVectorsFormat;
Review Comment:
@xiangfu0 / @Jackie-Jiang, this codec can not be used to write data since now it's in `backward_codec` package. unit test `LoaderTest.testVectorIndexLoad:766->constructSegmentWithVectorIndex:928` is failing with the following exception
```bash
Caused by: java.lang.UnsupportedOperationException: Old codecs may only be used for reading
at org.apache.lucene.backward_codecs.lucene95.Lucene95HnswVectorsFormat.fieldsWriter(Lucene95HnswVectorsFormat.java:182)
```
based on the [conversation](https://lists.apache.org/thread/fl9ds5ofxgmq2obzh0j5hgsmp4n6jx65), I found, is it okay if I use lucene99 based codecs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #12866:
URL: https://github.com/apache/pinot/pull/12866#issuecomment-2088309617
@Jackie-Jiang / @xiangfu0, Can you please review it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #12866:
URL: https://github.com/apache/pinot/pull/12866
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559174589
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneFSTIndexReader.java:
##########
@@ -52,7 +52,8 @@ public LuceneFSTIndexReader(PinotDataBuffer pinotDataBuffer)
_dataBufferIndexInput = new PinotBufferIndexInput(_dataBuffer, 0L, _dataBuffer.size());
_readFST =
- new FST(_dataBufferIndexInput, _dataBufferIndexInput, PositiveIntOutputs.getSingleton(), new OffHeapFSTStore());
+ new FST<>(FST.readMetadata(_dataBufferIndexInput, PositiveIntOutputs.getSingleton()),
Review Comment:
Introduced new static class [FSTMetadata](https://lucene.apache.org/core/9_9_0/core/org/apache/lucene/util/fst/FST.FSTMetadata.html) in Lucene 9.9.0. This is getting used in the FST constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #12866:
URL: https://github.com/apache/pinot/pull/12866#issuecomment-2076188853
> I don't fully follow why did it break. Following the same logic in #11857 should work. Basically when downgrading and Pinot doesn't understand the new format, it should try to generate the old format
I think, in this case, it's failing because we are generating the [segment based on updated](https://github.com/apache/pinot/blob/99a41803305a39887805e1106f90d07e4b6af978/pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/SegmentOp.java#L183) Lucene library-based code using codec99, and for [uploading](https://github.com/apache/pinot/blob/99a41803305a39887805e1106f90d07e4b6af978/pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/SegmentOp.java#L222) using the old library based code. So while, uploading we are getting this error
```bash
2024/04/17 02:28:04.193 INFO [StartServiceManagerCommand] [Start a Pinot [SERVER]] Started Pinot [SERVER] instance [Server_10.1.0.40_8098] at 9.332s since launch
2024/04/17 02:38:12.482 ERROR [LuceneTextIndexReader] [HelixTaskExecutor-message_handle_thread_4] Failed to instantiate Lucene text index reader for column textDim1, exception Could not load codec 'Lucene99'. Did you forget to add lucene-backward-codecs.jar?
2024/04/17 02:38:12.482 ERROR [ImmutableSegmentLoader] [HelixTaskExecutor-message_handle_thread_4] Failed to load segment: FeatureTest1_Segment with SegmentDirectory
java.lang.RuntimeException: java.lang.IllegalArgumentException: Could not load codec 'Lucene99'. Did you forget to add lucene-backward-codecs.jar?
at org.apache.pinot.segment.local.segment.index.readers.text.LuceneTextIndexReader.<init>(LuceneTextIndexReader.java:101) ~[pinot-segment-local-1.2.0-compat-1713320271.jar:1.2.0-compat-1713320271-1d807df40160ec8525b5a33847d67a61fef2c54e]
```
Do you think it's a valid reason? Also, I think generation of the segment in the compatibility test should also be based on the previous version itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559175866
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/vector/lucene95/HnswCodec.java:
##########
@@ -18,15 +18,15 @@
*/
package org.apache.pinot.segment.local.segment.creator.impl.vector.lucene95;
+import org.apache.lucene.backward_codecs.lucene90.Lucene90PostingsFormat;
Review Comment:
Lucene has moved these codecs to `backward_codecs` package.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1841269269
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneFSTIndexReader.java:
##########
@@ -52,7 +52,8 @@ public LuceneFSTIndexReader(PinotDataBuffer pinotDataBuffer)
_dataBufferIndexInput = new PinotBufferIndexInput(_dataBuffer, 0L, _dataBuffer.size());
_readFST =
- new FST(_dataBufferIndexInput, _dataBufferIndexInput, PositiveIntOutputs.getSingleton(), new OffHeapFSTStore());
+ new FST<>(FST.readMetadata(_dataBufferIndexInput, PositiveIntOutputs.getSingleton()),
Review Comment:
what about the OffheapFSTStore?
In recent tests, I found the default became OnheapFSTStore.
```
public FST(FSTMetadata<T> metadata, DataInput in) throws IOException {
this(metadata, new OnHeapFSTStore(DEFAULT_MAX_BLOCK_BITS, in, metadata.numBytes));
}
```
I'm looking for how to initialize it with OffheapFSTStore. Feel free to chime in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1841269269
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneFSTIndexReader.java:
##########
@@ -52,7 +52,8 @@ public LuceneFSTIndexReader(PinotDataBuffer pinotDataBuffer)
_dataBufferIndexInput = new PinotBufferIndexInput(_dataBuffer, 0L, _dataBuffer.size());
_readFST =
- new FST(_dataBufferIndexInput, _dataBufferIndexInput, PositiveIntOutputs.getSingleton(), new OffHeapFSTStore());
+ new FST<>(FST.readMetadata(_dataBufferIndexInput, PositiveIntOutputs.getSingleton()),
Review Comment:
what about the OffheapFSTStore?
In recent tests, I found the default became OnheapFSTStore.
```
public FST(FSTMetadata<T> metadata, DataInput in) throws IOException {
this(metadata, new OnHeapFSTStore(DEFAULT_MAX_BLOCK_BITS, in, metadata.numBytes));
}
```
I'm looking for how to initialize it with OffheapFSTStore. Feel free to chime in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12866:
URL: https://github.com/apache/pinot/pull/12866#issuecomment-2047162999
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12866?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `8 lines` in your changes are missing coverage. Please review.
> Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`4b80af1`)](https://app.codecov.io/gh/apache/pinot/pull/12866?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 258 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12866?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ache/pinot/segment/local/utils/fst/FSTBuilder.java](https://app.codecov.io/gh/apache/pinot/pull/12866?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Futils%2Ffst%2FFSTBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9mc3QvRlNUQnVpbGRlci5qYXZh) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12866?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...al/segment/index/readers/LuceneFSTIndexReader.java](https://app.codecov.io/gh/apache/pinot/pull/12866?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Freaders%2FLuceneFSTIndexReader.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvTHVjZW5lRlNUSW5kZXhSZWFkZXIuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12866?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12866 +/- ##
=============================================
- Coverage 61.75% 0.00% -61.76%
=============================================
Files 2436 2422 -14
Lines 133233 132712 -521
Branches 20636 20543 -93
=============================================
- Hits 82274 0 -82274
- Misses 44911 132712 +87801
+ Partials 6048 0 -6048
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.63%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.75%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-61.76%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12866?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on PR #12866:
URL: https://github.com/apache/pinot/pull/12866#issuecomment-2067871551
I am seeing a similar discussion that happened last time while upgrading the Lucene library, [PR](https://github.com/apache/pinot/pull/11857).
Compatability tests are failing for failing to load `lucene99Codec` class. @xiangfu0 / @Jackie-Jiang please suggest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #12866:
URL: https://github.com/apache/pinot/pull/12866#issuecomment-2079819764
We can use the same mechanism in #11857 to avoid this backward incompatibility by introducing a new suffix for the lucene99 index
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559170172
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/FSTBuilder.java:
##########
@@ -36,28 +36,31 @@
*/
public class FSTBuilder {
public static final Logger LOGGER = LoggerFactory.getLogger(FSTBuilder.class);
- private final FSTCompiler<Long> _builder = new FSTCompiler<>(FST.INPUT_TYPE.BYTE4, PositiveIntOutputs.getSingleton());
+ private final FSTCompiler<Long> _fstCompiler =
+ (new FSTCompiler.Builder<>(FST.INPUT_TYPE.BYTE4, PositiveIntOutputs.getSingleton())).build();
private final IntsRefBuilder _scratch = new IntsRefBuilder();
public static FST<Long> buildFST(SortedMap<String, Integer> input)
throws IOException {
PositiveIntOutputs fstOutput = PositiveIntOutputs.getSingleton();
- FSTCompiler<Long> fstCompiler = new FSTCompiler<>(FST.INPUT_TYPE.BYTE4, fstOutput);
+ FSTCompiler.Builder<Long> fstCompilerBuilder = new FSTCompiler.Builder<>(FST.INPUT_TYPE.BYTE4, fstOutput);
+ FSTCompiler<Long> fstCompiler = fstCompilerBuilder.build();
IntsRefBuilder scratch = new IntsRefBuilder();
for (Map.Entry<String, Integer> entry : input.entrySet()) {
fstCompiler.add(Util.toUTF16(entry.getKey(), scratch), entry.getValue().longValue());
}
- return fstCompiler.compile();
+
+ return FST.fromFSTReader(fstCompiler.compile(), fstCompiler.getFSTReader());
Review Comment:
This [change](https://github.com/apache/lucene/commit/63d4ba938fe0a4a82e615dd66e716684016cfc32) in Lucene mentions about getting an FST object from the FSTCompiler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559174589
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneFSTIndexReader.java:
##########
@@ -52,7 +52,8 @@ public LuceneFSTIndexReader(PinotDataBuffer pinotDataBuffer)
_dataBufferIndexInput = new PinotBufferIndexInput(_dataBuffer, 0L, _dataBuffer.size());
_readFST =
- new FST(_dataBufferIndexInput, _dataBufferIndexInput, PositiveIntOutputs.getSingleton(), new OffHeapFSTStore());
+ new FST<>(FST.readMetadata(_dataBufferIndexInput, PositiveIntOutputs.getSingleton()),
Review Comment:
Introduced new static class [FSTMetadata](https://lucene.apache.org/core/9_9_0/core/org/apache/lucene/util/fst/FST.FSTMetadata.html) in lucene 9.9.0. This is getting used in FST constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Upgrade lucene to 9.10.0 and compatibility changes to code. [pinot]
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #12866:
URL: https://github.com/apache/pinot/pull/12866#discussion_r1559826386
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/vector/lucene95/HnswVectorsFormat.java:
##########
@@ -19,10 +19,10 @@
package org.apache.pinot.segment.local.segment.creator.impl.vector.lucene95;
import java.io.IOException;
+import org.apache.lucene.backward_codecs.lucene95.Lucene95HnswVectorsFormat;
Review Comment:
> Does this mean after upgrading Lucece to 0.9.10:
>
> 1. We can only write index with lucene99 based codecs?
True, after upgrading Lucece to 0.9.10, we can only write an index based on lucene99.
> 2. If a new generated index is served by an old server (running Lucene 9.8.0), will it break?
I am unsure, but I will say it will break if I have to guess. The only thing is this vector functionality (classes) got added in Pinot on Dec/23, so I am not sure how much it is getting used. Maybe @xiangfu0 can help us here with a better understanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org