gibheer
/
bleve
0
0
Fork 0
Code

scorch fix disjunction searcher Advance() 

Found with "versus" test (TestScorchVersusUpsideDownBoltSmallMNSAM),
which had a boolean query with a MustNot that was the same as the Must
parameters.  This replicates a situation found by
Aruna/Mihir/testrunner/RQG (MB-27291).  Example:

  "query": {
    "must_not": {"disjuncts": [
      {"field": "body", "match": "hello"}
    ]},
    "must": {"conjuncts": [
      {"field": "body", "match": "hello"}
    ]}
  }

The nested searchers along the MustNot pathway would end up looking
roughly like...

  booleanSearcher
    MustNot
      => disjunctionSearcher
         => disjunctionSearcher
            => termSearcher

On the first Next() call by the collector, the two disjunction
searchers would run through their respective Next() method processing,
which includes their initSearcher() processing on the first time.
This has the effect of driving the leaf termSearcher through two
Next() invocations.

That is, if there were 3 docs (doc-1, doc-2, doc-3), the leaf
termSearcher would at this point have moved to point to doc-3, while
the topmost MustNot would have received doc-1.

Next, the booleanSearcher's Must searcher would produce doc-2, so the
booleanSearcher would try to Advance() the MustNot searcher to doc-2.

But, in scorch, the leafmost termSearcher had already gotten past
doc-2 and would return its doc-3.

In upsidedown, in contrast, the leaf termSearcher would then drive the
KVStore iterator with a Seek(doc-2), and the KVStore iterator would
perform a backwards seek to reach doc-2.

In scorch, however, backwards iteration seeking isn't supported.

So, this fix checks the state of the disjunction searcher to see if we
already have the necessary state so that we don't have to perform
actual Advance()'es on the underlying searchers.  This not only fixes
the behavior w.r.t. scorch, but also can have an effect of potentially
making upsidedown slightly faster as we're avoiding some backwards
KVStore iterator seeks.
This commit is contained in:
Steve Yen 2017-12-21 17:49:55 -08:00
parent 93c787ca09
commit d425a3be86
2 changed files with 10 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
search/searcher/search_disjunction.go
View File

@ -223,6 +223,9 @@ func (s *DisjunctionSearcher) Advance(ctx *search.SearchContext,
var err error
for i, searcher := range s.searchers {
if s.currs[i] != nil {
if s.currs[i].IndexInternalID.Compare(ID) >= 0 {
continue
}
ctx.DocumentMatchPool.Put(s.currs[i])
}
s.currs[i], err = searcher.Advance(ctx, ID)

14
test/versus_test.go
View File

@ -44,11 +44,11 @@ import (
func TestScorchVersusUpsideDownBolt(t *testing.T) {
(&VersusTest{
t: t,
NumDocs: 10000,
NumDocs: 1000,
MaxWordsPerDoc: 20,
NumWords: 100,
BatchSize: 100,
NumAttemptsPerSearch: 1000,
NumWords: 10,
BatchSize: 10,
NumAttemptsPerSearch: 100,
}).run(scorch.Name, boltdb.Name, upsidedown.Name, boltdb.Name, nil)
}
@ -138,7 +138,7 @@ var searchTemplates = []string{
}
}`,
`{
"about": "must-not-same-as-must -- FAILS!!!",
"about": "must-not-same-as-must -- see: MB-27291",
"query": {
"must_not": {"disjuncts": [
{"field": "body", "match": "{{word}}"}
@ -149,7 +149,7 @@ var searchTemplates = []string{
}
}`,
`{
"about": "must-not-same-as-should -- FAILS!!!",
"about": "must-not-same-as-should",
"query": {
"must_not": {"disjuncts": [
{"field": "body", "match": "{{word}}"}
@ -160,7 +160,7 @@ var searchTemplates = []string{
}
}`,
`{
"about": "inspired by testrunner RQG issue -- FAILS!!!",
"about": "inspired by testrunner RQG issue -- see: MB-27291",
"query": {
"must_not": {"disjuncts": [
{"field": "title", "match": "Trista Allen"},