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

add highlight max analyzed offset config to avoid long text discover search fail #191384

Conversation

iamgd67
Copy link

@iamgd67 iamgd67 commented Aug 27, 2024

add highlight max analyzed offset config to avoid long text discover search fail
resolves #159391

Summary

es default allow 1000000 text highlight anylyzes, if longer than this limit, discover will return fail. it's much better just ignore too long text full highlight than fail the search. so add a config and a default 1000000.

see https://www.elastic.co/guide/en/elasticsearch/reference/7.17/highlighting.html#offsets-strategy

image

For maintainers

@iamgd67 iamgd67 requested review from a team as code owners August 27, 2024 08:00
Copy link

cla-checker-service bot commented Aug 27, 2024

💚 CLA has been signed

@iamgd67 iamgd67 closed this Aug 27, 2024
@iamgd67 iamgd67 reopened this Aug 27, 2024
@iamgd67 iamgd67 requested a review from nickofthyme September 2, 2024 02:43
@kertal kertal requested a review from lukasolson September 9, 2024 15:17
@lukasolson
Copy link
Member

Thanks for contributing!

I'm discussing some things internally on how to move forward with this PR, but until then I just wanted to let you know we're actively thinking about this and give some initial feedback:

Having one advanced setting for this specific parameter is probably not what we want moving forward. As you've pointed out, there are other highlight-related parameters that users/admins may want to specifically configure. So we might want to consider changing this to accept a JSON object that can specify multiple parameters. (This is similar to how query:queryString:options works.)

I would hold off on making additional changes on this PR for now, as I'm trying to get the Elasticsearch folks involved as well. What honestly makes most sense to me is to default the max_analyzed_offset value to the same value that is stored in the index settings (index.highlight.max_analyzed_offset). But this change would probably make more sense for Elasticsearch to make rather than at the Kibana level. I could also be missing some context here, so I just want to make sure we're all on the same page.

Again, thanks for contributing... We recognize that the current behavior is not ideal, and we need to do something to help out here.

@nickofthyme nickofthyme removed their request for review September 18, 2024 13:56
@lukasolson
Copy link
Member

We brought up this need with the Elasticsearch team and they have added elastic/elasticsearch#112822. If they implement this, we can just default to setting it to whatever the index is configured to.

If that is fixed, would you still have a need to configure a value to something different?

@iamgd67
Copy link
Author

iamgd67 commented Sep 24, 2024

We brought up this need with the Elasticsearch team and they have added elastic/elasticsearch#112822. If they implement this, we can just default to setting it to whatever the index is configured to.

If that is fixed, would you still have a need to configure a value to something different?

No, I agree that's better. thanks all, I will close this.

@iamgd67 iamgd67 closed this Sep 24, 2024
@iamgd67
Copy link
Author

iamgd67 commented Sep 24, 2024

for thoese who need a workaround, here is a quick and dirty way. tested in docker image kibana:7.17.8, other version should work too.
sed -i 's/,fragment_size:m/\0 ,max_analyzed_offset:1000000/' src/plugins/field_formats/target/public/fieldFormats.plugin.js && gzip -k -f src/plugins/field_formats/target/public/fieldFormats.plugin.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possibility to configure max_analyzed_offset in highlight request
4 participants