Rescore collapsed documents by fred84 · Pull Request #28521 · elastic/elasticsearch

fred84 · 2018-02-05T15:20:46Z

Add support for rescoring collapsed docs (#27243). Documents at first get collapsed and then rescored.

@jimczi please take a look

…th_rescore

…84/elasticsearch into 27243_collapse_with_rescore

elasticmachine · 2018-02-05T15:20:47Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2018-02-05T15:20:47Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

jimczi · 2018-02-14T09:49:15Z

@elasticmachine ok to test

jimczi

It looks good to me.
I'll merge if the build passes with the changes, thanks @fred84 !

fred84 · 2018-02-14T11:22:19Z

@jimczi

https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml#L241 is failing. I think we could remove it, I've already added integration test for this behaviour

jimczi

Thanks @fred84, you can remove the failing tests, it is no longer needed. Though I left some comments regarding the IT test. I think it needs to be changed to ensure that scoring and ordering are consistent.

jimczi · 2018-02-14T12:40:53Z

server/src/test/java/org/elasticsearch/search/functionscore/QueryRescorerIT.java

+
+        SearchResponse searchResponse = client().prepareSearch("test")
+            .setTypes("type1")
+            .setQuery(new MatchQueryBuilder("name", "one"))


The score of this query depends on the number of shards, the default similarity, ... To make sure that we have consistent scoring you can use a function_score query like the following:

QueryBuilder query = functionScoreQuery( termQuery("name", "one"), ScoreFunctionBuilders.fieldValueFactorFunction("my_static_doc_score") ).boostMode(CombineFunction.REPLACE);

... and add the my_static_doc_score at indexing time.

jimczi · 2018-02-14T12:40:54Z

server/src/test/java/org/elasticsearch/search/functionscore/QueryRescorerIT.java

+        SearchResponse searchResponse = client().prepareSearch("test")
+            .setTypes("type1")
+            .setQuery(new MatchQueryBuilder("name", "one"))
+            .addRescorer(new QueryRescorerBuilder(new MatchQueryBuilder("name", "two")))


You can use the same for the rescore with another field for instance

fred84 · 2018-02-15T15:48:50Z

@jimczi Thanks for reviewing. I'll update PR next week.

…84/elasticsearch into 27243_collapse_with_rescore

fred84 · 2018-02-27T08:13:58Z

@jimczi PR updated, now integration test use static scoring.

jimczi · 2018-02-28T17:20:17Z

@elasticmachine ok to test

jimczi

Thanks @fred84

This change adds the ability to rescore collapsed documents.

* master: [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (elastic#28927) Remove BytesRef usage from XContentParser and its subclasses (elastic#28792) [DOCS] Correct typo in configuration (elastic#28903) Fix incorrect datemath example (elastic#28904) Add a usage example of the JLH score (elastic#28905) Wrap stream passed to createParser in try-with-resources (elastic#28897) Rescore collapsed documents (elastic#28521) Fix (simple)_query_string to ignore removed terms (elastic#28871) [Docs] Fix typo in composite aggregation (elastic#28891) Try if tombstone is eligable for pruning before locking on it's key (elastic#28767)

This reverts commit f057fc2. The rescorer does not resort the collapsed values inside the top docs during rescoring. For this reason the Lucene rescorer is not compatible with collapsing. Relates #27243

jimczi · 2018-03-08T10:23:54Z

I had to revert this change since it doesn't work as expected. I forgot that the collapsed values would also need to be resorted by the rescorer. We use these values in the coordinating node to collapse the results of each shard but the rescorer in Lucene cannot access them:

elasticsearch/server/src/main/java/org/apache/lucene/search/grouping/CollapseTopFieldDocs.java

Line 41 in 99f88f1

public final Object[] collapseValues;

I am really sorry I missed that but since it would require a rewriting of the rescorer in Lucene and that the collapsing code is only in es I don't think it is worth the effort.

rpedela · 2018-03-08T13:20:16Z

Doesn't Solr support collapse + rescore (rerank)? The claim that Lucene's rescorer needs a rewrite seems dubious.

jimczi · 2018-03-08T14:07:08Z

I agree that we should be able to rescore collapsed documents but this is more high hanging fruit than I thought which is why I reverted and closed the issue for now (sorry @fred84 ).
The current design of the collapsing in es is not compatible with the rescorer and will require some internal refactoring. I've started to work on this refactoring and when it's ready we'll reevaluate this pr if @fred84 still wants to work on it ;).

fred84 · 2018-03-08T15:44:41Z

@jimczi let me now when I can start this issue again :)

* es/master: (48 commits) Update bucket-sort-aggregation.asciidoc (#28937) [Docs] REST high-level client: Fix code for most basic search request (#28916) Improved percolator's random candidate query duel test and fixed bugs that were exposed by this: Revert "Rescore collapsed documents (#28521)" Build: Fix test logger NPE when no tests are run (#28929) [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (#28927) Remove BytesRef usage from XContentParser and its subclasses (#28792) [DOCS] Correct typo in configuration (#28903) Fix incorrect datemath example (#28904) Add a usage example of the JLH score (#28905) Wrap stream passed to createParser in try-with-resources (#28897) Rescore collapsed documents (#28521) Fix (simple)_query_string to ignore removed terms (#28871) [Docs] Fix typo in composite aggregation (#28891) Try if tombstone is eligable for pruning before locking on it's key (#28767) Limit analyzed text for highlighting (improvements) (#28808) Missing `timeout` parameter from the REST API spec JSON files (#28328) Clarifies how query_string splits textual part (#28798) Update outdated java version reference (#28870) ...

* es/6.x: (48 commits) Update bucket-sort-aggregation.asciidoc (#28937) [Docs] REST high-level client: Fix code for most basic search request (#28916) Improved percolator's random candidate query duel test and fixed bugs that were exposed by this: Revert "Rescore collapsed documents (#28521)" Build: Fix test logger NPE when no tests are run (#28929) [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (#28927) Remove BytesRef usage from XContentParser and its subclasses (#28792) Add doc note for -server flag on Windows service [DOCS] Correct typo in configuration (#28903) Fix incorrect datemath example (#28904) Add a usage example of the JLH score (#28905) Limit analyzed text for highlighting (improvements) (#28907) Wrap stream passed to createParser in try-with-resources (#28897) [Docs] Fix typo in composite aggregation (#28891) Rescore collapsed documents (#28521) Fix (simple)_query_string to ignore removed terms (#28871) Missing `timeout` parameter from the REST API spec JSON files (#28328) Clarifies how query_string splits textual part (#28798) Update outdated java version reference (#28870) ...

This change adds the ability to rescore collapsed documents.

This reverts commit f057fc2. The rescorer does not resort the collapsed values inside the top docs during rescoring. For this reason the Lucene rescorer is not compatible with collapsing. Relates elastic#27243

jimczi · 2018-03-30T13:18:14Z

Sorry it took me some time to come back at this. I checked why Solr was able to rescore the collapsed documents seamlessly and found out that they force the routing of each group in a single shard. This means that all the documents belonging to a single group are on the same shard so the rescoring is always done on the final head of the group. In es we don't enforce the routing so each group can be spread over multiple shards. This complicates the rescoring since it is always applied at the shard level and in this case on the temporary head of the groups (we don't know the final head in the shard since another shard can contain a better document for that group). For this reason I am reluctant to add this functionality because it might be surprising to see a head in a group that is not the best document of that group in the final response. This can happen if the rescoring gives a score to a document in a shard that is better that the score of the best document in the group which is in another shard. I don't see how we could avoid this unless we force the routing of the groups.

kgmonkhin · 2020-04-16T17:58:26Z

Is there a way to revisit this functionality? This limitation curtails the use of any LTR algorithms

DajunZhou · 2020-05-08T12:21:20Z

Excuse me, is there a way to apply rescoring before collapsing？

lasagar · 2020-05-08T22:21:02Z

Is there a way to revisit this functionality? This limitation curtails the use of any LTR algorithms

I am looking for this option for LTR rescoring

hosuaby · 2020-07-20T10:01:23Z

Hello, @jimczi

I am looking for possible (even limited) solution to make collapse working with rescore (especially LTR).
For the moment, I created a small plugin, that support search extension:

"ext": {
    "post-rescore" : { /* ... */ }
}

This post-rescore has the same syntax as original rescore, but it is executed as FetchSubPhase (so I assume, that it's executed on coordinating node).

The present solution rescore only results present on current page.

Code of solution is here:
https://github.com/hosuaby/bug-elasticsearch-collapse-rescore/tree/master/src/main/java/io/hosuaby/elasticsearch

I will be thankful for any feedback and suggestions.

Morikko · 2020-08-13T09:39:17Z

@hosuaby you might have a look to this discussion: #60946 it looks close to your proposition

fred84 added 11 commits January 30, 2018 21:48

rescore in collapse top doc collector

90cb54e

Merge remote-tracking branch 'upstream/master' into 27243_collapse_wi…

a770c25

…th_rescore

Merge remote-tracking branch 'upstream/master' into 27243_collapse_wi…

e235174

…th_rescore

test for rescore + collapse

e737519

collapse + rescore test

0b91413

Merge remote-tracking branch 'upstream/master' into 27243_collapse_wi…

40fca97

…th_rescore

Merge remote-tracking branch 'upstream/master' into 27243_collapse_wi…

5663f10

…th_rescore

Merge branch '27242_collapse_with_rescore' of https://github.com/fred…

a5d8834

…84/elasticsearch into 27243_collapse_with_rescore

clean diff

e060df4

fix checkstyle warnings

4f5729a

fix test

cc8f8ef

jimczi added >enhancement :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.3.0 labels Feb 14, 2018

jimczi approved these changes Feb 14, 2018

View reviewed changes

jimczi requested changes Feb 14, 2018

View reviewed changes

fred84 added 3 commits February 26, 2018 19:20

Merge branch 'master' into 27243_collapse_with_rescore

12e4d06

Merge branch '27243_collapse_with_rescore' of https://github.com/fred…

c5151b5

…84/elasticsearch into 27243_collapse_with_rescore

remove rest test + static score in integration test

8320622

jimczi approved these changes Mar 4, 2018

View reviewed changes

jimczi merged commit f057fc2 into elastic:master Mar 4, 2018

jimczi pushed a commit that referenced this pull request Mar 4, 2018

Rescore collapsed documents (#28521)

c60c699

This change adds the ability to rescore collapsed documents.

fred84 deleted the 27243_collapse_with_rescore branch March 5, 2018 06:30

dakrone mentioned this pull request Mar 7, 2018

QueryRescorerIT.testRescoreAfterCollapse fails reproducibly #28932

Closed

jasontedor mentioned this pull request Mar 7, 2018

support field collapsing + rescore #27243

Closed

jimczi added won't fix and removed v6.3.0 v7.0.0 labels Mar 8, 2018

jimczi removed the won't fix label Mar 8, 2018

sebasjm pushed a commit to sebasjm/elasticsearch that referenced this pull request Mar 10, 2018

Rescore collapsed documents (elastic#28521)

5821de0

This change adds the ability to rescore collapsed documents.

jimczi added the stalled label Mar 14, 2018

Morikko mentioned this pull request Aug 13, 2020

Add rerank phase in coordinate node after reduced #60946

Closed

Conversation

fred84 commented Feb 5, 2018

Uh oh!

elasticmachine commented Feb 5, 2018

Uh oh!

elasticmachine commented Feb 5, 2018

Uh oh!

jimczi commented Feb 14, 2018

Uh oh!

jimczi left a comment

Choose a reason for hiding this comment

Uh oh!

fred84 commented Feb 14, 2018

Uh oh!

jimczi left a comment

Choose a reason for hiding this comment

Uh oh!

jimczi Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

fred84 Feb 27, 2018

Choose a reason for hiding this comment

Uh oh!

jimczi Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

fred84 Feb 27, 2018

Choose a reason for hiding this comment

Uh oh!

fred84 commented Feb 15, 2018

Uh oh!

fred84 commented Feb 27, 2018

Uh oh!

jimczi commented Feb 28, 2018

Uh oh!

jimczi left a comment

Choose a reason for hiding this comment

Uh oh!

jimczi commented Mar 8, 2018

Uh oh!

rpedela commented Mar 8, 2018

Uh oh!

jimczi commented Mar 8, 2018

Uh oh!

fred84 commented Mar 8, 2018

Uh oh!

jimczi commented Mar 30, 2018

Uh oh!

kgmonkhin commented Apr 16, 2020

Uh oh!

DajunZhou commented May 8, 2020

Uh oh!

lasagar commented May 8, 2020

Uh oh!

hosuaby commented Jul 20, 2020

Uh oh!

Morikko commented Aug 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants