Fix disabling of bulk scoring in monitor (#16350)

The monitor wraps all queries in a ForceNoBulkScoringQuery to ensure
that it runs everything doc-at-a-time and doesn't allocate large
bulk buffers for searches over a tiny number of documents.  Changes
to the bulk scoring API in Weight and ScorerSupplier meant that this
optimization was silently disabled.

Add a new test to ensure that bulk scoring is properly disabled,
and re-work ForceNoBulkScoringQuery so that it behaves as expected.

Author: romseygeek

lucene/CHANGES.txt

@@ -327,6 +327,7 @@ Optimizations
 
 Bug Fixes
 ---------------------
+* GITHUB#16350: Disable bulk-scoring in monitor queries. (Alan Woodward)
 
 * GITHUB#16295: SingletonSortedNumericDocValues now delegates rangeIntoBitSet
   to the wrapped NumericDocValues, enabling optimized range evaluation for

lucene/monitor/src/java/org/apache/lucene/monitor/ForceNoBulkScoringQuery.java

@@ -20,12 +20,14 @@
 import java.io.IOException;
 import java.util.Objects;
 import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.BulkScorer;
 import org.apache.lucene.search.Explanation;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.Matches;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.QueryVisitor;
 import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
 import org.apache.lucene.search.ScorerSupplier;
 import org.apache.lucene.search.Weight;
 
@@ -87,7 +89,29 @@ public Explanation explain(LeafReaderContext leafReaderContext, int i) throws IO
 
       @Override
       public ScorerSupplier scorerSupplier(LeafReaderContext leafReaderContext) throws IOException {
-        return innerWeight.scorerSupplier(leafReaderContext);
+        ScorerSupplier innerScorerSupplier = innerWeight.scorerSupplier(leafReaderContext);
+        if (innerScorerSupplier == null) {
+          return null;
+        }
+        return new ScorerSupplier() {
+
+          @Override
+          public Scorer get(long leadCost) throws IOException {
+            return innerScorerSupplier.get(leadCost);
+          }
+
+          @Override
+          public long cost() {
+            return innerScorerSupplier.cost();
+          }
+
+          @Override
+          public BulkScorer bulkScorer() throws IOException {
+            // force the use of the default doc-by-doc BulkScorer, rather than any
+            // optimized bulk-scoring implementation the wrapped query may provide
+            return new DefaultBulkScorer(get(Long.MAX_VALUE));
+          }
+        };
       }
 
       @Override

lucene/monitor/src/test/org/apache/lucene/monitor/TestForceNoBulkScoringQuery.java

@@ -26,10 +26,19 @@
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.Term;
+import org.apache.lucene.search.BulkScorer;
+import org.apache.lucene.search.FilterWeight;
+import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.ScorerSupplier;
 import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.Weight;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.util.LuceneTestCase;
 
@@ -73,4 +82,106 @@ public void testRewrite() throws IOException {
       }
     }
   }
+
+  public void testBulkScoringIsDisabled() throws IOException {
+
+    try (Directory dir = newDirectory();
+        IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(new StandardAnalyzer()))) {
+
+      Document doc = new Document();
+      doc.add(new TextField("field", "term1 term2 term3 term4", Field.Store.NO));
+      iw.addDocument(doc);
+      iw.commit();
+
+      try (IndexReader reader = DirectoryReader.open(dir)) {
+        IndexSearcher searcher = newSearcher(reader);
+        // disable query caching, so that we exercise the search path directly rather than
+        // any bulk-scoring optimizations the cache may apply internally
+        searcher.setQueryCache(null);
+
+        Query throwing = new ThrowsOnBulkScoreQuery(new TermQuery(new Term("field", "term1")));
+
+        // sanity check that the wrapped query really does throw when bulk-scored directly
+        expectThrows(AssertionError.class, () -> searcher.search(throwing, 10));
+
+        Query wrapped = new ForceNoBulkScoringQuery(throwing);
+        // should not throw, because bulk scoring has been disabled
+        assertEquals(1, searcher.search(wrapped, 10).totalHits.value());
+      }
+    }
+  }
+
+  /**
+   * A query wrapper whose ScorerSupplier throws an AssertionError if bulkScorer() is called, used
+   * to check that {@link ForceNoBulkScoringQuery} never calls bulkScorer() on the ScorerSupplier of
+   * the query it wraps.
+   */
+  private static class ThrowsOnBulkScoreQuery extends Query {
+
+    private final Query in;
+
+    private ThrowsOnBulkScoreQuery(Query in) {
+      this.in = in;
+    }
+
+    @Override
+    public Query rewrite(IndexSearcher indexSearcher) throws IOException {
+      Query rewritten = in.rewrite(indexSearcher);
+      if (rewritten != in) {
+        return new ThrowsOnBulkScoreQuery(rewritten);
+      }
+      return super.rewrite(indexSearcher);
+    }
+
+    @Override
+    public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost)
+        throws IOException {
+      Weight innerWeight = in.createWeight(searcher, scoreMode, boost);
+      return new FilterWeight(innerWeight) {
+        @Override
+        public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+          ScorerSupplier innerSupplier = super.scorerSupplier(context);
+          if (innerSupplier == null) {
+            return null;
+          }
+          return new ScorerSupplier() {
+            @Override
+            public Scorer get(long leadCost) throws IOException {
+              return innerSupplier.get(leadCost);
+            }
+
+            @Override
+            public long cost() {
+              return innerSupplier.cost();
+            }
+
+            @Override
+            public BulkScorer bulkScorer() {
+              throw new AssertionError("bulkScorer() should not have been called");
+            }
+          };
+        }
+      };
+    }
+
+    @Override
+    public String toString(String field) {
+      return "ThrowsOnBulkScore(" + in.toString(field) + ")";
+    }
+
+    @Override
+    public void visit(QueryVisitor visitor) {
+      in.visit(visitor);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      return sameClassAs(o) && in.equals(((ThrowsOnBulkScoreQuery) o).in);
+    }
+
+    @Override
+    public int hashCode() {
+      return classHash() * 31 + in.hashCode();
+    }
+  }
 }