[GitHub] [solr] alessandrobenedetti commented on a change in pull request #8: SOLR-14607: LTR Query, the TimeAllowed parameter causes a timeout exception after no results

[GitBox <gi...@apache.org>]

alessandrobenedetti commented on a change in pull request #8:
URL: https://github.com/apache/solr/pull/8#discussion_r594462247



##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -124,10 +125,13 @@ public TopDocs rescore(IndexSearcher searcher, TopDocs firstPassTopDocs,
     final LTRScoringQuery.ModelWeight modelWeight = (LTRScoringQuery.ModelWeight) searcher
         .createWeight(searcher.rewrite(scoringQuery), ScoreMode.COMPLETE, 1);
 
-    scoreFeatures(searcher,topN, modelWeight, firstPassResults, leaves, reranked);
+    int hitUpto = scoreFeatures(searcher,topN, modelWeight, firstPassResults, leaves, reranked);
+    final ScoreDoc[] rerankHited = new ScoreDoc[hitUpto];

Review comment:
       rerankHited ?
   -> rerankedHits ?

##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -124,10 +125,13 @@ public TopDocs rescore(IndexSearcher searcher, TopDocs firstPassTopDocs,
     final LTRScoringQuery.ModelWeight modelWeight = (LTRScoringQuery.ModelWeight) searcher
         .createWeight(searcher.rewrite(scoringQuery), ScoreMode.COMPLETE, 1);
 
-    scoreFeatures(searcher,topN, modelWeight, firstPassResults, leaves, reranked);
+    int hitUpto = scoreFeatures(searcher,topN, modelWeight, firstPassResults, leaves, reranked);
+    final ScoreDoc[] rerankHited = new ScoreDoc[hitUpto];
+    System.arraycopy(reranked, 0, rerankHited,0, hitUpto);

Review comment:
       are we introducing a copy of the reranked for?

##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -181,14 +185,19 @@ public void scoreFeatures(IndexSearcher indexSearcher,
         readerContext = leaves.get(readerUpto);
         endDoc = readerContext.docBase + readerContext.reader().maxDoc();
       }
-      // We advanced to another segment
-      if (readerContext != null) {
-        docBase = readerContext.docBase;
-        scorer = modelWeight.scorer(readerContext);
+      try{
+        // We advanced to another segment
+        if (readerContext != null) {
+          docBase = readerContext.docBase;
+          scorer = modelWeight.scorer(readerContext);
+        }
+        scoreSingleHit(indexSearcher, topN, modelWeight, docBase, hitUpto, hit, docID, scoringQuery, scorer, reranked);
+        hitUpto++;
+      } catch (ExitableDirectoryReader.ExitingReaderException ex) {
+        break;

Review comment:
       I was reasoning about this, I think I prefer an all or nothing approach:
   Try to rerank up to the topK I am targeting, in the allowed time, if I fail to succeed in the allowed time I don't rerank at all.
   Having some hit rescored and others not, because of time, it's not ideal, given the fact the reRankDocs should regulate what is reranked and what not




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org