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

Fixing the Engine specific files from INDEX_STORE_HYBRID_NIO_EXTENSIONS list to make sure engine files are getting mmapped #1054

Closed
wants to merge 2 commits into from

Conversation

navneet1v
Copy link
Collaborator

@navneet1v navneet1v commented Aug 23, 2023

Description

This PR includes:

  1. Fixing the Engine specific files from INDEX_STORE_HYBRID_NIO_EXTENSIONS list to make sure engine files are getting mmapped.
  2. Fixing the backward incompatible changes in ScoreScript class. [Feature] Expose term frequency in Painless script score context OpenSearch#9081

For 1, Core team introduced a change which resulted in deprecating the setting: INDEX_STORE_HYBRID_MMAP_EXTENSIONS, hence as the setting was deprecated the unit tests were failing with error, as warnings were getting generated.

2> REPRODUCE WITH: ./gradlew ':test' --tests "org.opensearch.knn.index.KNNSettingsTests.testGetSettingValueDefault" -Dtests.seed=45E93CD93802AAC7 -Dtests.security.manager=false -Dtests.locale=ar-LB -Dtests.timezone=Asia/Tehran -Druntime.java=11
  2> java.lang.AssertionError:
    Expected: a collection containing "[index.store.hybrid.mmap.extensions] setting was deprecated in OpenSearch and will be removed in a future release! See the breaking changes documentation for the next major version."

With this change we are removing the .vec and .vex file(Engine specific files) from INDEX_STORE_HYBRID_NIO_EXTENSIONS to make sure that .vec and .vex files are getting mmaped. As per the documentation , if file extensions are not present in INDEX_STORE_HYBRID_NIO_EXTENSIONS they will be Mmapped.

Files list on node after the change

"index": {
					"store": {
						"hybrid": {
							"nio": {
								"extensions": [
									"segments_N",
									"write.lock",
									"si",
									"cfe",
									"fnm",
									"fdx",
									"fdt",
									"pos",
									"pay",
									"nvm",
									"dvm",
									"tvx",
									"tvd",
									"liv",
									"dii",
									"vem"
								]
							}
						}
					}
				},

Files list on node before the change


Issues Resolved

#995

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --signoff

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.

…NS list to make sure engine files are getting mmaped.

Signed-off-by: Navneet Verma <navneev@amazon.com>
@navneet1v navneet1v added the Maintenance Add support for new versions of OpenSearch/Dashboards from upstream label Aug 23, 2023
…ipt class

Signed-off-by: Navneet Verma <navneev@amazon.com>
Copy link

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

The changes for Fixing the backward incompatible changes in ScoreScript class. https://github.com/opensearch-project/OpenSearch/pull/9081 looks good to me

@navneet1v navneet1v closed this Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Add support for new versions of OpenSearch/Dashboards from upstream skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants