SERVER-34454 Consider including the EOF bonus in the plan cache score

Removes the subtraction of the eofBonus from the final score,
thus including the eofBonus in the plan cache score.

This is safe to do because the cached score is used only for logging
purposes and no decision is made base on the score outside of
plan_ranker::pickBestPlan

3 files changed, 13 insertions, 35 deletions

diff --git a/src/mongo/db/exec/plan_cache_util.h b/src/mongo/db/exec/plan_cache_util.h
index 717f3c879ef..8f0260409d7 100644
--- a/src/mongo/db/exec/plan_cache_util.h
+++ b/src/mongo/db/exec/plan_cache_util.h
@@ -99,7 +99,7 @@ void updatePlanCache(
         // In "sometimes cache" mode, we cache unless we hit one of the special cases below.
         canCache = true;
 
-        if (ranking->tieForBest) {
+        if (ranking->tieForBest()) {
             // The winning plan tied with the runner-up and we're using "sometimes cache" mode. We
             // will not write a plan cache entry.
             canCache = false;
diff --git a/src/mongo/db/query/plan_ranker_util.h b/src/mongo/db/query/plan_ranker_util.h
index 58bcc16d3df..8818c47dd22 100644
--- a/src/mongo/db/query/plan_ranker_util.h
+++ b/src/mongo/db/query/plan_ranker_util.h
@@ -155,14 +155,6 @@ StatusWith<std::unique_ptr<PlanRankingDecision>> pickBestPlan(
         why->stats = StatsDetails{};
     }
 
-    // Determine whether plans tied for the win.
-    if (scoresAndCandidateIndices.size() > 1U) {
-        double bestScore = scoresAndCandidateIndices[0].first;
-        double runnerUpScore = scoresAndCandidateIndices[1].first;
-        const double epsilon = 1e-10;
-        why->tieForBest = std::abs(bestScore - runnerUpScore) < epsilon;
-    }
-
     // Update results in 'why'
     // Stats and scores in 'why' are sorted in descending order by score.
     auto&& stats = why->getStats<PlanStageStatsType>();
@@ -171,26 +163,6 @@ StatusWith<std::unique_ptr<PlanRankingDecision>> pickBestPlan(
         double score = scoresAndCandidateIndices[i].first;
         size_t candidateIndex = scoresAndCandidateIndices[i].second;
 
-        // We shouldn't cache the scores with the EOF bonus included, as this is just a
-        // tie-breaking measure for plan selection. Plans not run through the multi plan runner
-        // will not receive the bonus.
-        //
-        // An example of a bad thing that could happen if we stored scores with the EOF bonus
-        // included:
-        //
-        //   Let's say Plan A hits EOF, is the highest ranking plan, and gets cached as such. On
-        //   subsequent runs it will not receive the bonus. Eventually the plan cache feedback
-        //   mechanism will evict the cache entry - the scores will appear to have fallen due to
-        //   the missing EOF bonus.
-        //
-        // This raises the question, why don't we include the EOF bonus in scoring of cached plans
-        // as well? The problem here is that the cached plan runner always runs plans to completion
-        // before scoring. Queries that don't get the bonus in the multi plan runner might get the
-        // bonus after being run from the plan cache.
-        if (statTrees[candidateIndex]->common.isEOF) {
-            score -= eofBonus;
-        }
-
         stats.candidatePlanStats.push_back(std::move(statTrees[candidateIndex]));
         why->scores.push_back(score);
         why->candidateOrder.push_back(candidateIndex);
diff --git a/src/mongo/db/query/plan_ranking_decision.h b/src/mongo/db/query/plan_ranking_decision.h
index b5c7cbf6f67..8e8543b98e3 100644
--- a/src/mongo/db/query/plan_ranking_decision.h
+++ b/src/mongo/db/query/plan_ranking_decision.h
@@ -137,6 +137,18 @@ struct PlanRankingDecision {
         return stdx::get<typename StatsToDetailMap<PlanStageStatsType>::Value>(stats);
     }
 
+    /*
+     * Returns true if there are at least two possible plans, and at least the top two plans
+     * have the same scores.
+     */
+    bool tieForBest() const {
+        if (scores.size() > 1) {
+            const double epsilon = 1e-10;
+            return (std::abs(scores[0] - scores[1]) < epsilon);
+        }
+        return false;
+    }
+
     // Execution stats details for each candidate plan.
     stdx::variant<StatsDetails, SBEStatsDetails> stats;
 
@@ -158,11 +170,5 @@ struct PlanRankingDecision {
     //
     // Like 'candidateOrder', the contents of this array are indicies into the 'candidates' array.
     std::vector<size_t> failedCandidates;
-
-    // Whether two plans tied for the win.
-    //
-    // Reading this flag is the only reliable way for callers to determine if there was a tie,
-    // because the scores kept inside the PlanRankingDecision do not incorporate the EOF bonus.
-    bool tieForBest = false;
 };
 }  // namespace mongo::plan_ranker