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.

Sign up for GitHub

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

Jump to bottom

Shrink slow log for rank_feature query #83847

Merged
merged 1 commit into from
Feb 11, 2022
Merged

Shrink slow log for rank_feature query #83847

merged 1 commit into from
Feb 11, 2022

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Feb 11, 2022

This removes the boost from the toXContent of rank_feature if it
is the default. It also removes the score function if it is the default.

Relates to #76515

@nik9000 nik9000 added >non-issue :Search/Search Search-related issues that do not fall into other categories v8.2.0 labels Feb 11, 2022
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Feb 11, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@nik9000
Shrink slow log for rank_feature query
464d55d 
This removes the `boost` from the `toXContent` of `rank_feature` if it
is the default. It also removes the score function if it is the default.

Relates to elastic#76515
jtibshirani
jtibshirani reviewed Feb 11, 2022
View reviewed changes
Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

This makes sense to me! Is the plan to audit all the query types and avoid printing defaults? Also I was curious if we considered consulting include_defaults from XContent.Params like we do for mappings?

@nik9000
Copy link
Member Author

nik9000 commented Feb 11, 2022 •
edited
Loading

is makes sense to me! Is the plan to audit all the query types and avoid printing defaults?

That was my plan, yeah.

Also I was curious if we considered consulting include_defaults from XContent.Params like we do for mappings?

No one mentioned that parameter specifically when we were talking about it. At the time we did talk about making it something a parameter could do and enable it for the slow log but we couldn't think of anything else that converted queries into xcontent. At least, we couldn't think of anything that cared about the defaults.

@nik9000
Copy link
Member Author

nik9000 commented Feb 11, 2022

** jtibshirani ** approved these changes 11 minutes ago

Thanks!

@nik9000 nik9000 merged commit 4fddf98 into elastic:master Feb 11, 2022
@nik9000 nik9000 mentioned this pull request Feb 17, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtibshirani jtibshirani jtibshirani approved these changes

@mayya-sharipova mayya-sharipova Awaiting requested review from mayya-sharipova

Assignees
No one assigned
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nik9000 @elasticmachine @jtibshirani