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

Allow mmap to use new preview JDK-19 APIs in Apache Lucene 9.4+ #5151

Merged
merged 2 commits into from
Nov 8, 2022

Conversation

reta
Copy link
Collaborator

@reta reta commented Nov 8, 2022

Signed-off-by: Andriy Redko [email protected]

Description

Allow mmap to use new preview JDK-19 APIs in Apache Lucene 9.4+. When --enable-preview is not present:

[2022-11-08T14:03:56,502][WARN ][o.a.l.s.MMapDirectory    ] [runTask-0] You are running with Java 19. To make full use of MMapDirectory, please pass '--enable-preview' to the Java command line.

With --enable-preview:

[2022-11-08T14:10:36,009][INFO ][o.a.l.s.MemorySegmentIndexInputProvider] [runTask-0] Using MemorySegmentIndexInput with Java 19

Issues Resolved

Final part of #4637

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@reta reta requested a review from a team as a code owner November 8, 2022 19:13
@reta reta added the v3.0.0 Issues and PRs related to version 3.0.0 label Nov 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

CHANGELOG.md Outdated
@@ -135,6 +135,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Fail weight update when decommission ongoing and fail decommission when attribute not weighed away ([#4839](https://github.com/opensearch-project/OpenSearch/pull/4839))
- Skip SymbolicLinkPreservingTarIT when running on Windows ([#5023](https://github.com/opensearch-project/OpenSearch/pull/5023))
- Change the output error message back to use OpenSearchException in the cause chain. ([#5081](https://github.com/opensearch-project/OpenSearch/pull/5081))
- Allow mmap to use new preview JDK-19 APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
Copy link
Member

Choose a reason for hiding this comment

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

We just pushed an update to the changelog policy that applies the learnings from the recent 2.4 release.

This is probably an example that can skip the changelog? Whatever changes make use of the new APIs would be relevant, but this is probably too low level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @andrross , I could be mistaken but it falls into A newly added feature: we activate a new Apache Lucene feature by changing the JVM parameters, what do you think from this perspective? (I am fine removing changelog line as well)

Copy link
Member

@andrross andrross Nov 8, 2022

Choose a reason for hiding this comment

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

Ah, does Lucene detect this setting and change its behavior at runtime accordingly? If so then you're right :)

I should have read the description more carefully. You're right this is a newly added feature.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @andrross !

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@reta reta force-pushed the allow.preview branch 2 times, most recently from 2869696 to f381e50 Compare November 8, 2022 19:54
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@uschindler
Copy link
Contributor

Technically the preview feature would only work with exactly java 19. But it won't hurt if you pass it with 20. Lucenes will ignore it.

@reta
Copy link
Collaborator Author

reta commented Nov 8, 2022

Technically the preview feature would only work with exactly java 19. But it won't hurt if you pass it with 20. Lucenes will ignore it.

Thanks @uschindler , I started with 19:--enable-preview (exact) but replaced it with 19-:--enable-preview

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

javadoc.options.addBooleanOption("-enable-preview", true)
javadoc.options.addStringOption("-release", BuildParams.runtimeJavaVersion.majorVersion)
}
javadoc.options.addStringOption("-release", targetCompatibility.majorVersion)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Align javadoc release with target compatibility settings

@reta reta requested a review from dblock November 8, 2022 21:14
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #5151 (a046505) into main (fab4336) will increase coverage by 0.09%.
The diff coverage is 66.34%.

@@             Coverage Diff              @@
##               main    #5151      +/-   ##
============================================
+ Coverage     70.84%   70.94%   +0.09%     
- Complexity    57867    58141     +274     
============================================
  Files          4692     4708      +16     
  Lines        276650   277557     +907     
  Branches      40155    40188      +33     
============================================
+ Hits         196002   196921     +919     
+ Misses        64500    64482      -18     
- Partials      16148    16154       +6     
Impacted Files Coverage Δ
...java/org/opensearch/upgrade/ValidateInputTask.java 84.61% <0.00%> (ø)
...ch/analysis/common/CommonAnalysisModulePlugin.java 92.33% <0.00%> (+0.05%) ⬆️
...mon/HyphenationCompoundWordTokenFilterFactory.java 0.00% <0.00%> (ø)
...index/analysis/IcuCollationTokenFilterFactory.java 59.25% <0.00%> (-2.28%) ⬇️
.../src/main/java/org/opensearch/LegacyESVersion.java 59.34% <ø> (-2.37%) ⬇️
...ecommission/awareness/put/DecommissionRequest.java 85.71% <0.00%> (-6.60%) ⬇️
...min/cluster/stats/TransportClusterStatsAction.java 70.83% <ø> (ø)
...ensearch/action/search/SearchTransportService.java 83.75% <ø> (-3.25%) ⬇️
...va/org/opensearch/action/update/UpdateRequest.java 46.72% <0.00%> (+2.28%) ⬆️
...arch/cluster/decommission/DecommissionService.java 35.27% <0.00%> (ø)
... and 634 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dblock dblock merged commit fc47572 into opensearch-project:main Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants