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

LUCENE-8996: maxScore is sometimes missing from distributed responses #906

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

diegoceccarelli
Copy link
Contributor

@diegoceccarelli diegoceccarelli commented Sep 28, 2019

Description

This is an issue in Lucene that affects Solr Distributed Grouped responses: TopGroupsShardResponseProcessor uses TopGroups.merge to merge together the grouped results of different shard. If a shard didn't have results for a particular group, it will return an empty group with maxScore == Float.NaN, merge just performs Math.max that is going to return Float.NaN if one of the two float is Float.NaN.

The issue was open in 2017 by Julien Massenet, I experienced it while working on #300 so I updated his solution to the upstream version.

Solution

The PR adds a function:

private static float updateMaxScore(float currentMaxScore, float newMaxScore)

that will implement a max that works with Float.NaN (same

Tests

A unit test covering all the different combinations of groups that could be processed by merge is provided.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I am authorized to contribute this code to the ASF and have removed any code I do not have a license to distribute.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the master branch.
  • I have run ant precommit and the appropriate test suite.
  • I have added tests for my changes.
  • I have added documentation for the Ref Guide (for Solr changes only).

@diegoceccarelli diegoceccarelli changed the title SOLR-8441: maxScore is sometimes missing from distributed responses LUCENE-8996: maxScore is sometimes missing from distributed responses Oct 4, 2019
@mikemccand
Copy link
Member

Hmm, I see this src fix was committed, but the new unit test was not committed -- was that intentional?

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

Successfully merging this pull request may close these issues.

2 participants