Add support for matched_fields with the unified highlighter #18166

nomoa · 2025-04-30T09:58:57Z

Description

This option was silently ignored when running the unified highlighter. Since lucene 10 it is now possible to pass a list of additional fields to fetch matches from and blend into the snippets.

Related Issues

Resolves #18164

Check List

Functionality includes testing.
~~[ ] API changes companion pull request created, if applicable.~~
Public documentation issue/PR: Document that the unified highlighter supports matched_field documentation-website#9793

github-actions · 2025-04-30T10:11:25Z

❌ Gradle check result for e9a0a85: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2025-04-30T11:37:02Z

❌ Gradle check result for ad13011: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2025-04-30T14:00:33Z

❌ Gradle check result for 6f75191: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

nomoa · 2025-04-30T14:20:30Z

Last CI failure is:
org.opensearch.upgrades.FullClusterRestartIT.testRecovery:

java.lang.AssertionError: expected at least 1 old segment. segments:
p 0 testrecovery 10.1.0
r 0 testrecovery 10.1.0
. Actual: 0
	at __randomizedtesting.SeedInfo.seed([6D0A5D888E677F10:ACFA2424A337B5B7]:0)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failEquals(Assert.java:187)
	at org.junit.Assert.assertNotEquals(Assert.java:201)
	at org.opensearch.upgrades.FullClusterRestartIT.testRecovery(FullClusterRestartIT.java:782)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:938)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:974)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:988)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
[...]

Which I believe is unrelated to this patch?

github-actions · 2025-05-02T19:43:08Z

❌ Gradle check result for 6f75191: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2025-05-03T13:08:25Z

❌ Gradle check result for 32061e5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

nomoa · 2025-05-05T12:20:44Z

Failure I can see from the last build is:

> Task :test:fixtures:gcs-fixture:composeDown
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-third-party_1           ... 
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture_1                       ... 
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-other_1                 ... 
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-repositories-metering_1 ... 
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-repositories-metering_1 ... error
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture_1                       ... error
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-third-party_1           ... error
Stopping c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-other_1                 ... done

ERROR: for c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-repositories-metering_1  cannot stop container: 6be4186fdfda17109af09d376297dc67310ec862c6c4d760714806ab0172e90a: container 6be4186fdfda PID 37320 is zombie and can not be killed. Use the --init option when creating containers to run an init inside the container that forwards signals and reaps processes

ERROR: for c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture_1  cannot stop container: 1bfd96106dd37b667377653973bd6207eff410e6689127693cde6e6e1bf7645a: container 1bfd96106dd3 PID 37378 is zombie and can not be killed. Use the --init option when creating containers to run an init inside the container that forwards signals and reaps processes

ERROR: for c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-third-party_1  cannot stop container: 2c9315a4c36fa8f299580844e98e5576d7b5f57dcc255d3628f6acc31724938a: container 2c9315a4c36f PID 37470 is zombie and can not be killed. Use the --init option when creating containers to run an init inside the container that forwards signals and reaps processes
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-third-party_1           ... 
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture_1                       ... 
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-other_1                 ... 
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-repositories-metering_1 ... 
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-other_1                 ... done
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture_1                       ... done
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-third-party_1           ... done
Removing c632c20032974ac355b722657aedda3e_gcs-fixture_gcs-fixture-repositories-metering_1 ... done
Removing network c632c20032974ac355b722657aedda3e_gcs-fixture_default

> Task :test:fixtures:krb5kdc-fixture:composeDown
Stopping 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_peppa_1 ... 
Stopping 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_hdfs_1  ... 
Stopping 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_peppa_1 ... done
Stopping 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_hdfs_1  ... done
Removing 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_peppa_1 ... 
Removing 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_hdfs_1  ... 
Removing 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_hdfs_1  ... done
Removing 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_peppa_1 ... done
Removing network 2ff70b1d4a8f216a2e2075eb5fa05c77_krb5kdc-fixture_default
build complete, generating: /var/jenkins/workspace/gradle-check/search/build/57600.tar.bz2

[Incubating] Problems report is available at: file:///var/jenkins/workspace/gradle-check/search/build/reports/problems/problems-report.html

FAILURE: Build failed with an exception.

github-actions · 2025-05-05T12:48:55Z

❌ Gradle check result for a6fbda7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2025-05-08T19:18:49Z

✅ Gradle check result for fa503d1: SUCCESS

codecov · 2025-05-08T19:19:12Z

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.

Project coverage is 72.42%. Comparing base (560ac10) to head (fa503d1).
Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
...h/fetch/subphase/highlight/UnifiedHighlighter.java	63.63%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #18166      +/-   ##
============================================
- Coverage     72.56%   72.42%   -0.15%     
+ Complexity    67261    67228      -33     
============================================
  Files          5476     5482       +6     
  Lines        310478   310674     +196     
  Branches      45133    45156      +23     
============================================
- Hits         225313   224993     -320     
- Misses        66840    67359     +519     
+ Partials      18325    18322       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

msfroh

This looks great! Thanks a lot @nomoa!

andrross · 2025-05-08T19:48:54Z

...rnalClusterTest/java/org/opensearch/search/fetch/subphase/highlight/HighlighterSearchIT.java

+                .setMapping(
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("properties")
+                        .startObject("foo")
+                        .field("type", "text")
+                        .field("store", true)
+                        .field("analyzer", "mock_english")
+                        .startObject("fields")
+                        .startObject("plain")
+                        .field("type", "text")
+                        .field("analyzer", "standard")
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                )


Super minor suggestion, more for future reference and not something you need to change...Java now allows multiline strings so you can just write regular JSON to make this a bit more readable:

Suggested change

.setMapping(

XContentFactory.jsonBuilder()

.startObject()

.startObject("properties")

.startObject("foo")

.field("type", "text")

.field("store", true)

.field("analyzer", "mock_english")

.startObject("fields")

.startObject("plain")

.field("type", "text")

.field("analyzer", "standard")

.endObject()

.endObject()

.endObject()

.endObject()

.endObject()

)

.setMapping("""

{

"properties": {

"foo": {

"type": "text",

"store": true

"analyzer": "mock_english"

"fields": {

"plain": {

"type": "text"

"analyzer": "standard"

}

}

}

}

}

""")

good call, thanks for the suggestion, if time permits I might try to clean this whole test class in a separate PR if you find this useful.

nomoa · 2025-05-09T17:33:34Z

is the code coverage check being green mandatory? The code is being tested in integration tests but if you prefer I can introduce a dedicated test for these lines (might need to make some methods visible for testing, unless there are some facilities to easily create/mock FieldHighlightContext)

This option was silently ignored when running the unified highlighter. Since lucene 10 it is now possible to pass a list of additional fields to fetch matches from and blend into the snippets. Signed-off-by: David Causse <[email protected]>

github-actions · 2025-05-09T20:39:52Z

❌ Gradle check result for 4d6a2ae: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

nomoa requested review from anasalkouz, andrross, ashking94, Bukhtawar, CEHENKLE, cwperks, dbwiddis, gbbafna, jed326, kotwanikunal, mch2, msfroh, nknize, owaiskazi19, reta, Rishikesh1159, sachinpkale, saratvemulapalli, shwetathareja, sohami, VachaShah and a team as code owners April 30, 2025 09:58

github-actions bot added enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc labels Apr 30, 2025

nomoa force-pushed the unified-highlighter-support-matched-fields branch from e9a0a85 to ad13011 Compare April 30, 2025 10:39

This was referenced Apr 29, 2025

[AUTOCUT] Gradle Check Flaky Test Report for CreateIndexIT #14312

Open

[AUTOCUT] Gradle Check Flaky Test Report for WarmIndexSegmentReplicationIT #18157

Open

nomoa force-pushed the unified-highlighter-support-matched-fields branch from ad13011 to 6f75191 Compare April 30, 2025 13:05

nomoa mentioned this pull request Apr 30, 2025

Document that the unified highlighter supports matched_field opensearch-project/documentation-website#9793

Open

1 task

opensearch-ci-bot mentioned this pull request Apr 30, 2025

[AUTOCUT] Gradle Check Flaky Test Report for FullClusterRestartIT #15813

Open

nomoa force-pushed the unified-highlighter-support-matched-fields branch 3 times, most recently from f7907c5 to 32061e5 Compare May 3, 2025 12:41

nomoa force-pushed the unified-highlighter-support-matched-fields branch from 32061e5 to a6fbda7 Compare May 5, 2025 12:23

This was referenced May 6, 2025

[AUTOCUT] Gradle Check Flaky Test Report for S3BlobStoreRepositoryTests #14299

Open

[AUTOCUT] Gradle Check Flaky Test Report for S3BlobContainerRetriesTests #17551

Open

nomoa force-pushed the unified-highlighter-support-matched-fields branch from a6fbda7 to fa503d1 Compare May 8, 2025 18:09

msfroh approved these changes May 8, 2025

View reviewed changes

andrross reviewed May 8, 2025

View reviewed changes

andrross approved these changes May 8, 2025

View reviewed changes

nomoa force-pushed the unified-highlighter-support-matched-fields branch from fa503d1 to 4d6a2ae Compare May 9, 2025 20:06

This was referenced May 10, 2025

[AUTOCUT] Gradle Check Flaky Test Report for QueueResizableOpenSearchThreadPoolExecutorTests #14297

Open

[AUTOCUT] Gradle Check Flaky Test Report for SecureReactorNetty4HttpServerTransportTests #17486

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for matched_fields with the unified highlighter #18166

Add support for matched_fields with the unified highlighter #18166

nomoa commented Apr 30, 2025 •

edited

Loading

github-actions bot commented Apr 30, 2025

github-actions bot commented Apr 30, 2025

github-actions bot commented Apr 30, 2025

nomoa commented Apr 30, 2025 •

edited

Loading

github-actions bot commented May 2, 2025

github-actions bot commented May 3, 2025

nomoa commented May 5, 2025

github-actions bot commented May 5, 2025

github-actions bot commented May 8, 2025

codecov bot commented May 8, 2025

msfroh left a comment

andrross May 8, 2025

nomoa May 9, 2025 •

edited

Loading

nomoa commented May 9, 2025

github-actions bot commented May 9, 2025

Add support for matched_fields with the unified highlighter #18166

Are you sure you want to change the base?

Add support for matched_fields with the unified highlighter #18166

Conversation

nomoa commented Apr 30, 2025 • edited Loading

Description

Related Issues

Check List

github-actions bot commented Apr 30, 2025

github-actions bot commented Apr 30, 2025

github-actions bot commented Apr 30, 2025

nomoa commented Apr 30, 2025 • edited Loading

github-actions bot commented May 2, 2025

github-actions bot commented May 3, 2025

nomoa commented May 5, 2025

github-actions bot commented May 5, 2025

github-actions bot commented May 8, 2025

codecov bot commented May 8, 2025

Codecov Report

msfroh left a comment

Choose a reason for hiding this comment

andrross May 8, 2025

Choose a reason for hiding this comment

nomoa May 9, 2025 • edited Loading

Choose a reason for hiding this comment

nomoa commented May 9, 2025

github-actions bot commented May 9, 2025

nomoa commented Apr 30, 2025 •

edited

Loading

nomoa commented Apr 30, 2025 •

edited

Loading

nomoa May 9, 2025 •

edited

Loading