Page MenuHomePhabricator

Evaluate result ordering change of incoming link counts
Closed, ResolvedPublic

Description

Scenario: Redirects count as incoming links                                                                                                                                     # features/relevancy_api.feature:22
  Given a page named Relevancyredirecttest Smaller exists with contents Relevancyredirecttest A text text text text text text text text text text text text text                # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Smaller/A exists with contents [[Relevancyredirecttest Smaller]]                                                                       # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Smaller/B exists with contents [[Relevancyredirecttest Smaller]]                                                                       # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Larger exists with contents Relevancyredirecttest B text text text text text text text text text text text text text                   # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Larger/Redirect exists with contents #REDIRECT [[Relevancyredirecttest Larger]]                                                        # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Larger/A exists with contents [[Relevancyredirecttest Larger]]                                                                         # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Larger/B exists with contents [[Relevancyredirecttest Larger/Redirect]]                                                                # features/step_definitions/page_steps.rb:1
  And a page named Relevancyredirecttest Larger/C exists with contents [[Relevancyredirecttest Larger/Redirect]]                                                                # features/step_definitions/page_steps.rb:1
  Then within 20 seconds api searching for Relevancyredirecttest yields Relevancyredirecttest Larger as the first result and Relevancyredirecttest Smaller as the second result # features/step_definitions/search_steps.rb:411
    expected ["Relevancyredirecttest Smaller/A"] to include "Relevancyredirecttest Smaller" (RSpec::Expectations::ExpectationNotMetError)

The result ordering has changed here, not sure if it's meaningful and the test should be changed or the code needs to change.

Examples:
es 1.7: http://cirrustest-cirrus-browser-bot.wmflabs.org/w/index.php?search=Relevancyredirecttest&title=Special:Search&go=Go
es 2.3: http://cirrustest-searchdemo.wmflabs.org/w/index.php?title=Special:Search&profile=default&fulltext=Search&search=Relevancyredirecttest

There is another perhaps related test about incoming links:

Scenario: Incoming links count in page weight                                                                                                                                               # features/relevancy_api.feature:65
  Given a page named Relevancylinktest Smaller exists                                                                                                                                       # features/step_definitions/page_steps.rb:1
  And a page named Relevancylinktest Larger Extraword exists                                                                                                                                # features/step_definitions/page_steps.rb:1
  And a page named Relevancylinktest Larger/Link A exists with contents [[Relevancylinktest Larger Extraword]]                                                                              # features/step_definitions/page_steps.rb:1
  And a page named Relevancylinktest Larger/Link B exists with contents [[Relevancylinktest Larger Extraword]]                                                                              # features/step_definitions/page_steps.rb:1
  And a page named Relevancylinktest Larger/Link C exists with contents [[Relevancylinktest Larger Extraword]]                                                                              # features/step_definitions/page_steps.rb:1
  And a page named Relevancylinktest Larger/Link D exists with contents [[Relevancylinktest Larger Extraword]]                                                                              # features/step_definitions/page_steps.rb:1
  When within 20 seconds api searching for Relevancylinktest -intitle:link yields Relevancylinktest Larger Extraword as the first result and Relevancylinktest Smaller as the second result # features/step_definitions/search_steps.rb:411
  And I api search with disabled incoming link weighting for Relevancylinktest -intitle:link                                                                                                # features/step_definitions/search_steps.rb:30
  Then Relevancylinktest Smaller is the first api search result                                                                                                                             # features/step_definitions/search_steps.rb:312
    expected ["Relevancylinktest Larger Extraword"] to include "Relevancylinktest Smaller" (RSpec::Expectations::ExpectationNotMetError)

Examples:
es 1.7: http://cirrustest-cirrus-browser-bot.wmflabs.org/w/index.php?title=Special:Search&profile=default&fulltext=Search&search=Relevancylinktest+-intitle%3Alink&cirrusBoostLinks=no&searchToken=36fv2gj89995esma1va1om29t
es 2.3: http://cirrustest-searchdemo.wmflabs.org/w/index.php?title=Special:Search&profile=default&fulltext=Search&search=Relevancylinktest+-intitle%3Alink&searchToken=1o8sejcpikcm5swfll65xvrg5&cirrusBoostLinks=no

Event Timeline

Restricted Application added projects: Discovery, Discovery-Search. · View Herald TranscriptApr 26 2016, 11:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I hope it's just because of the 'stats' param issue in the incLinks update job

I fixed the stats problem and reindexed, doesn't fix the problem. Double checked the action=cirrusdump output and all relevant properties are the same between both. Oddly on 1.7 both documents have the exact some score so the ordering is some random property of how it's calculated. On 2.3 the two documents have different scores. I haven't fully investigated but it looks like the structure of the scoring explanation is different, there are two more nesting levels on 2.3. This might not be fully relevant and a consequence of the filter -> query change.

I tried to dump out the index from 1.7 and then import it to 2.3, but getting a failed to derive xcontent error. Will spend some more time comparing the queries and expanations to see what's going on here.

Actually the top two links of the first failed test look to be right after fixing the stats part. The second query, with boost links disabled, is the only one still broken.

Not sure yet but after optimizing the indices on both install I can see that docFreq is 1 for Relevancylinktest Larger Extraword but 3 for Relevancylinktest Smaller. It means that the results are fetched from different shards but I'm afraid that's a bug in the explain since dfs_query_then_fetch should sum all the shards docFreq... It seems to be the case since both scores (not read from explain) are identical on searchdemo.

I think there's no bug here, the test is simply wrong I guess, Relevancylinktest Smaller and Relevancylinktest Larger Extraword cannot have different score (without boost links), if the purpose of the test was to test length normalisation we should add a lot more content to Relevancylinktest Larger Extraword because length norm is a very low precision attribute (8bits float) and both docs end up having the same lengthNorm (0.125, reported as fieldNorm in the explain).

Note that scores are not directly comparable between es 1.7 and es 2.x since they changed the internal query construct, by removing filtered queries the _type filter is now part of the scored query, it won't affect the ranking but it affects the queryNorm since queryNorm directly depends on the number of clauses... I'm not 100% sure but it's my best guess to explain why queryNorm is different...

Change 286426 had a related patch set uploaded (by DCausse):
Fix relevancy_api test with/without boostlinks

https://gerrit.wikimedia.org/r/286426

Change 286426 merged by jenkins-bot:
Fix relevancy_api test with/without boostlinks

https://gerrit.wikimedia.org/r/286426

debt closed this task as Resolved.Jun 8 2016, 12:30 AM
debt added a subscriber: debt.

Looks like this is resolved - closing.