Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16442: upgrade to Lucene 9.4 (do and document) #1056

Merged
merged 13 commits into from Oct 19, 2022

Conversation

cpoerschke
Copy link
Contributor

@cpoerschke cpoerschke commented Oct 7, 2022

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?
  • in SchemaCodecFactory lucene93 vs. lucene94 use?
  • properly read https://lucene.apache.org/core/9_4_0/MIGRATE.html

Comment on lines -40 to +53
* 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +552 to +556
@Override
public VectorEncoding vectorEncoding() {
return VectorEncoding.BYTE;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -522,6 +522,7 @@ private static class ReaderWrapper extends FilterLeafReader {
fieldInfo.getPointIndexDimensionCount(),
fieldInfo.getPointNumBytes(),
fieldInfo.getVectorDimension(),
fieldInfo.getVectorEncoding(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -26 to +28
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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpoerschke cpoerschke marked this pull request as ready for review October 7, 2022 17:34
@sonatype-lift
Copy link

sonatype-lift bot commented Oct 7, 2022

⚠️ 314 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

Copy link
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One thought on the docs, would it make sense to reference back to this PR and 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....

```

## Code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets read upgrading notes provided by Lucene first; ehh?
https://lucene.apache.org/core/9_4_0/MIGRATE.html

@cpoerschke cpoerschke marked this pull request as draft October 14, 2022 17:18
@dsmiley dsmiley marked this pull request as ready for review October 15, 2022 03:21
@cpoerschke
Copy link
Contributor Author

LGTM. One thought on the docs, would it make sense to reference back to this PR and 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

@cpoerschke cpoerschke merged commit 4ab7f4b into apache:main Oct 19, 2022
@cpoerschke cpoerschke deleted the lucene940 branch October 19, 2022 09:25
### `solr/licenses` update

```
git rm lucene-*-9.3.0.jar.sha1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./gradlew updateLicenses will handle this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! #1087 to simplify the docs (and I'll leave that PR open for a few days in case of additional documentation feedback)

asfgit pushed a commit that referenced this pull request Oct 19, 2022
(cherry picked from commit 4ab7f4b)

Resolved Conflicts:
	solr/solr-ref-guide/antora.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants