Fix race condition in GenerateAndCheckPerfResults

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Fixed
    • Priority: Major - P3
    • 9.0.0-rc0
    • Affects Version/s: None
    • Component/s: None
    • None
    • Product Performance
    • Fully Compatible
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      The benchmarks_sep instruction-count check in generate_and_check_perf_results.py silently fails to compare child-commit results against the parent commit when the parent's results have not yet been processed into the time_series/ API. The write path (perf-submission.sh/raw_perf_results/cedar_report) is asynchronous with respect to the read path (time_series/); a cron in the perf-monitoring service materialises time_series documents every five minutes, and that gap is widened by queue backlog. When the gap is hit, _get_base_commit_value(...) finds no data, logs an info-level "No time series data found …" line, and falls back to the latest run from an earlier commit (or returns None). Either way the gate runs against the wrong baseline.

      The race was confirmed on commit 1211fe8e: the original run failed to find parent data; a retry a few hours later on the same commit succeeded. Anjani Bhat confirmed there were benchmarks_sep results still sitting in time_series_updates at the time. Commit-queue merges are not blocked today, but the instruction-count regression check has effectively never been reliable: a real regression on a back-to-back commit will be silently missed.

      Desired behaviour

      The check should either:

      1. Use a read endpoint that does not depend on the 5-minute cron — e.g. the /raw_result endpoints Jeff Zambory pointed at (staging API docs), which expose data as soon as the parent task POSTs it; or
      2. Tell the perf-monitoring service to fast-track benchmarks_sep results — Anjani's suggestion of adding a priority field to /raw_perf_results/cedar_report and plumbing it through to time_series_updates.

      In all cases, "no data for parent commit" should be treated as a failure (or at least a loud, non-info-level warning) rather than silently falling back to the latest-run value. Today the fallback hides the bug.

      Implementation sketch

      Two changes are required regardless of which read strategy we pick:

      • Stop the silent fallback at generate_and_check_perf_results.py around line 339. When the parent commit value is not available, the hook should fail loudly so an engineer can re-run or investigate, instead of using the latest-run value as a baseline.
      • Decide between the /raw_result endpoint and the priority-field approach. The former is a mongodb/mongo only change; the latter needs coordination with the perf-monitoring-service team to add the field. Jeff/Anjani's input should drive that decision.

      A secondary optimisation Anjani mentioned: the sep_benchmark time series have grown long enough that change-point detection on them is slow, which contributes to queue backlog. Worth filing separately if we go with option 2.

      Additional Context


      Ticket originally authored by alice.doherty@mongodb.com, edited by james.littlejohn@mongodb.com during triage.

            Assignee:
            Alice Doherty
            Reporter:
            Alice Doherty
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: