You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/10/07 17:00:49 UTC
[GitHub] [solr] cpoerschke opened a new pull request, #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
cpoerschke opened a new pull request, #1056:
URL: https://github.com/apache/solr/pull/1056
https://issues.apache.org/jira/browse/SOLR-16442
Collaboration welcome e.g. open questions:
[ ] what to do about the `sortMissingLast` logic in the `SortableBinaryField` class?
[ ] what documentation needs changes for a Lucene dependency upgrade?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on PR #1056:
URL: https://github.com/apache/solr/pull/1056#issuecomment-1283667590
> LGTM. One thought on the docs, would it make sense to reference back to this PR and [SOLR-16442](https://issues.apache.org/jira/browse/SOLR-16442) for the next person to see an example? For repetitive tasks, it's great to have a nice model to follow that shows all the types of changes required....
I'd broken up the change into distinctive commits with that sort of use and potentially review ease in mind.
In terms of explicitly referencing back to the PR and/or JIRA issue, I'm not so convinced on that for two reasons:
* ideally the documentation should mention all that needs doing i.e. for the upgrade to be done based on documentation rather than past example(s)
* it might be off-putting or restrictive to see 13 commits i.e. working through a documented list incrementally is one thing but _perceiving_ that things should be done in separate commits might not be everyone's cup-of-tea
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r990331940
##########
solr/test-framework/src/java/org/apache/solr/schema/SortableBinaryField.java:
##########
@@ -70,12 +70,14 @@ public BinarySortField(final String field, final boolean reverse) {
field,
new FieldComparatorSource() {
@Override
- public FieldComparator.TermOrdValComparator newComparator(
+ public TermOrdValComparator newComparator(
final String fieldname,
final int numHits,
final boolean enableSkipping,
final boolean reversed) {
- return new FieldComparator.TermOrdValComparator(numHits, fieldname);
+ final boolean sortMissingLast = true; // TODO
Review Comment:
Looks like `false` is the existing behaviour: https://github.com/apache/lucene/blob/releases/lucene/9.3.0/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java#L284
```suggestion
final boolean sortMissingLast = false;
```
##########
solr/test-framework/src/java/org/apache/solr/schema/SortableBinaryField.java:
##########
@@ -23,9 +23,9 @@
import org.apache.lucene.document.SortedDocValuesField;
import org.apache.lucene.document.SortedSetDocValuesField;
import org.apache.lucene.index.IndexableField;
-import org.apache.lucene.search.FieldComparator;
import org.apache.lucene.search.FieldComparatorSource;
import org.apache.lucene.search.SortField;
+import org.apache.lucene.search.comparators.TermOrdValComparator;
Review Comment:
https://github.com/apache/lucene/commit/261db55806cd352520e406d5e5a684ce45afa9f4
##########
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java:
##########
@@ -522,6 +522,7 @@ private static class ReaderWrapper extends FilterLeafReader {
fieldInfo.getPointIndexDimensionCount(),
fieldInfo.getPointNumBytes(),
fieldInfo.getVectorDimension(),
+ fieldInfo.getVectorEncoding(),
Review Comment:
https://github.com/apache/lucene/blob/releases/lucene/9.4.0/lucene/core/src/java/org/apache/lucene/index/FieldInfo.java#L65-L86
##########
solr/core/src/java/org/apache/solr/core/MMapDirectoryFactory.java:
##########
@@ -37,20 +37,20 @@
* <li>unmap -- See {@link MMapDirectory#setUseUnmap(boolean)}
* <li>preload -- See {@link MMapDirectory#setPreload(boolean)}
* <li>maxChunkSize -- The Max chunk size. See {@link MMapDirectory#MMapDirectory(Path,
- * LockFactory, int)}
+ * LockFactory, long)}
* </ul>
*/
public class MMapDirectoryFactory extends StandardDirectoryFactory {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
boolean unmapHack;
boolean preload;
- private int maxChunk;
+ private long maxChunk;
@Override
public void init(NamedList<?> args) {
super.init(args);
SolrParams params = args.toSolrParams();
- maxChunk = params.getInt("maxChunkSize", MMapDirectory.DEFAULT_MAX_CHUNK_SIZE);
+ maxChunk = params.getLong("maxChunkSize", MMapDirectory.DEFAULT_MAX_CHUNK_SIZE);
Review Comment:
https://github.com/apache/lucene/blob/releases/lucene/9.4.0/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L117-L139
##########
solr/core/src/java/org/apache/solr/schema/SchemaField.java:
##########
@@ -548,6 +549,11 @@ public int vectorDimension() {
return 0;
}
+ @Override
+ public VectorEncoding vectorEncoding() {
+ return VectorEncoding.BYTE;
+ }
+
Review Comment:
https://github.com/apache/lucene/blob/releases/lucene/9.4.0/lucene/core/src/java/org/apache/lucene/index/IndexableFieldType.java#L104-L105
##########
solr/core/src/java/org/apache/solr/core/SchemaCodecFactory.java:
##########
@@ -23,9 +23,9 @@
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.KnnVectorsFormat;
import org.apache.lucene.codecs.PostingsFormat;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec.Mode;
-import org.apache.lucene.codecs.lucene92.Lucene92HnswVectorsFormat;
Review Comment:
I'm not sure why this changed from 91 to 92 in the 9.3 update in #960 but presume it should change to 94 for 9.4 here if possible?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r993156863
##########
solr/core/src/java/org/apache/solr/core/SchemaCodecFactory.java:
##########
@@ -23,9 +23,9 @@
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.KnnVectorsFormat;
import org.apache.lucene.codecs.PostingsFormat;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec.Mode;
-import org.apache.lucene.codecs.lucene92.Lucene92HnswVectorsFormat;
Review Comment:
Ah, thanks for that! docs-in-PR updated to account for 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r990314703
##########
solr/test-framework/src/java/org/apache/solr/schema/SortableBinaryField.java:
##########
@@ -70,12 +70,14 @@ public BinarySortField(final String field, final boolean reverse) {
field,
new FieldComparatorSource() {
@Override
- public FieldComparator.TermOrdValComparator newComparator(
+ public TermOrdValComparator newComparator(
final String fieldname,
final int numHits,
final boolean enableSkipping,
final boolean reversed) {
- return new FieldComparator.TermOrdValComparator(numHits, fieldname);
+ final boolean sortMissingLast = true; // TODO
Review Comment:
What to do (pun intended) about this?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
madrob commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r991429602
##########
solr/core/src/java/org/apache/solr/core/SchemaCodecFactory.java:
##########
@@ -23,9 +23,9 @@
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.KnnVectorsFormat;
import org.apache.lucene.codecs.PostingsFormat;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec.Mode;
-import org.apache.lucene.codecs.lucene92.Lucene92HnswVectorsFormat;
Review Comment:
The branch was called 9.2 because that's what I started on, but eventually moved on to 9.3 - there is nothing significant about the naming.
The codec is 92 because 93 didn't get new 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1056:
URL: https://github.com/apache/solr/pull/1056#issuecomment-1271877985
:warning: **314 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/solr/01GESP8Y0YGM7F7C9TWMZ9WFRA?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20solr) for more details.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r999187407
##########
dev-docs/lucene-upgrade.md:
##########
@@ -0,0 +1,93 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ -->
+
+# Lucene upgrade steps
+
+## Read
+
+https://lucene.apache.org/core/9_4_0/MIGRATE.html
+
+## Start
+
+Create a new branch locally e.g. `git checkout -b lucene940 -t origin/main` for upgrading to Lucene 9.4.0 version.
+
+## Build
+
+### `versions.props` update
+
+```
+- org.apache.lucene:*=9.3.0
++ org.apache.lucene:*=9.4.0
+```
+
+### `versions.lock` update
+
+```
+gradlew --write-locks
+```
+
+### `solr/licenses` update
+
+```
+git rm lucene-*-9.3.0.jar.sha1
Review Comment:
`./gradlew updateLicenses` will handle this
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r990344371
##########
solr/core/src/java/org/apache/solr/core/SchemaCodecFactory.java:
##########
@@ -23,9 +23,9 @@
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.KnnVectorsFormat;
import org.apache.lucene.codecs.PostingsFormat;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec;
-import org.apache.lucene.codecs.lucene92.Lucene92Codec.Mode;
-import org.apache.lucene.codecs.lucene92.Lucene92HnswVectorsFormat;
Review Comment:
Hmm, the #960 branch was called `lucene-9.2` and maybe non-use of 9.3 there was for a good reason?
Could this mean in turn that Lucene 9.4 for Solr 9.1 could be re-considered?
@chatman and @madrob - tagging you here as release manager and #960 author and then with more context the question could/should be brought to the dev list for wider consideration? WDYT?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r993876728
##########
dev-docs/lucene-upgrade.md:
##########
@@ -0,0 +1,89 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ -->
+
+# Lucene upgrade steps
+
+## Start
+
+Create a new branch locally e.g. `git checkout -b lucene940 -t origin/main` for upgrading to Lucene 9.4.0 version.
+
+## Build
+
+### `versions.props` update
+
+```
+- org.apache.lucene:*=9.3.0
++ org.apache.lucene:*=9.4.0
+```
+
+### `versions.lock` update
+
+```
+gradlew --write-locks
+```
+
+### `solr/licenses` update
+
+```
+git rm lucene-*-9.3.0.jar.sha1
+
+# ???manually get new .sha1 files???
+
+git add lucene-*-9.4.0.jar.sha1
+```
+
+## Code
+
Review Comment:
Lets read upgrading notes provided by Lucene first; ehh?
https://lucene.apache.org/core/9_4_0/MIGRATE.html
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke merged pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke merged PR #1056:
URL: https://github.com/apache/solr/pull/1056
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #1056: SOLR-16442: upgrade to Lucene 9.4 (do and document)
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #1056:
URL: https://github.com/apache/solr/pull/1056#discussion_r999197786
##########
dev-docs/lucene-upgrade.md:
##########
@@ -0,0 +1,93 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ -->
+
+# Lucene upgrade steps
+
+## Read
+
+https://lucene.apache.org/core/9_4_0/MIGRATE.html
+
+## Start
+
+Create a new branch locally e.g. `git checkout -b lucene940 -t origin/main` for upgrading to Lucene 9.4.0 version.
+
+## Build
+
+### `versions.props` update
+
+```
+- org.apache.lucene:*=9.3.0
++ org.apache.lucene:*=9.4.0
+```
+
+### `versions.lock` update
+
+```
+gradlew --write-locks
+```
+
+### `solr/licenses` update
+
+```
+git rm lucene-*-9.3.0.jar.sha1
Review Comment:
thanks! #1087 to simplify the docs (and I'll leave that PR open for a few days in case of additional documentation feedback)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org