Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Track performance history reset bugs: older runs refill reset series

Aperta
#511 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
48/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
github-actions, python
Ambito
ci-cd, performance

Direzione di ricerca

Start with scripts/nvx_tools/performance.py, especially persist_results and gate_results, then trace the persistence job and action in .github/workflows/ci.yml and .github/actions/persist-performance-results/action.yml. Read the selective-reset and idempotence tests in scripts/test_performance.py, then add coverage for an older result arriving after a reset. Done means nine post-reset points still report 9/10 warmup while unaffected metrics persist normally.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug-hunt confirmed

Hunt scope and revision

  • Repository: microsoft/nvx.
  • Latest origin/dev fetched at the start of this hunt: 9a2721a713a1bb78f0c94c189900cf89a728a26e (2026-10-12).
  • Focused areas: guest-time observation/time-ABI validation, and performance-gate baseline matching/history resets. Existing open and closed repo:microsoft/nvx label:bug-hunt issues were searched first; none covers the performance-history reset race below. #499 concerns stale suite logs before collection, not results arriving after a history reset.
  • Implementation and tests ran from an immutable git archive of that exact revision outside the worktree. No repository files were modified and no commits were created.
  • Investigation stopped at the first confirmed substantive bug, about 27 minutes into the 45-minute budget.

Confirmed finding: an older benchmark result can refill a just-reset history

Severity: Medium. Results measured before an intentional history reset can be appended to the new series by an older in-flight dev workflow. This mixes measurement regimes and can qualify the regression gate before its required ten new history points exist. The deterministic reproduction below changes the gate from the expected warmup/exit 0 to a regression/exit 1 with only nine post-reset points.

Trigger
  1. A successful dev workflow for commit A has measured its benchmark results, but has not persisted them yet.
  2. Commit B lands on dev, intentionally removing an affected metric from its tracked history, for example as part of a benchmark storage/platform change.
  3. Workflow A reaches Persist results. It fetches the latest branch, checks out B, then persists the CSVs already collected for A without checking whether the metric was reset after A.
  4. The A row is inserted into B's newly reset history. After nine genuine post-reset points, the gate sees ten rows and gates the metric instead of reporting Warmup (9/10).

The reset can be selective or a supported header-only reset. The reproduction uses a header-only history for one canonical platform/ABI/CPU series. Neither corrupt CSVs nor missing fields are required.

Expected versus actual

Expected: An artifact measured before a metric's explicit reset must not repopulate that reset series. Until ten matching post-reset measurements have been persisted, the metric remains a warmup and does not fail the gate. Unaffected metrics may still be persisted normally.

Actual: Persistence considers only dimensional metadata and whether the commit/metric identity currently exists in the destination CSV. A reset removes that identity, so the older row is treated as new and inserted. The gate then counts that pre-reset row toward its ten-point warmup.

Traced callers, implementation, and existing coverage

All source links are pinned to the hunt revision:

  • CI concurrency does not cancel in-flight dev pushes. A newer commit can land while an older run finishes.
  • Performance persistence job runs for successful pushes to dev; its checkout initially names that workflow's SHA.
  • Result preparation downloads the workflow's run-scoped artifacts and labels collected results with its GITHUB_SHA.
  • Persistence action then fetches and checks out the latest target branch before appending those older results. There is no reset-age/epoch check.
  • persist_results checks metadata and duplicate identities, but has no information about whether a source measurement predates a history reset.
  • gate_results counts the resulting history rows toward minimum_history.
  • Documented reset contract says that changing benchmark storage requires removing affected metrics so their ten-point warmup restarts rather than comparing new storage with the old baseline.
  • Existing selective-reset test persists a new result after a reset, not an older in-flight result. Existing idempotence coverage keeps the old identities present; it does not cover an identity removed by an intervening reset.
Small deterministic reproduction

Run this Python script with the working directory set to a checkout at the pinned revision. It uses the public CLI handlers and existing CSV writer, no mocks or hypervisor, and creates all CSVs in a temporary directory. The final assertion is an intentionally failing regression assertion.

import contextlib
import io
import sys
import tempfile
from pathlib import Path

sys.path.insert(0, str(Path.cwd() / "scripts"))
import nvx
from nvx_tools import performance

platform = "linux-kvm-virtual-machine"
filename = f"{platform}-microvm-v2-1vcpu.csv"
metric = "openvmm_snapshot_generation"
old_commit = "a" * 40

def row(commit, value):
    return performance.Result(commit, metric, "ms", "lower", value, platform, 2, 1)

with tempfile.TemporaryDirectory() as temporary:
    root = Path(temporary)
    source, history, target = (root / name for name in ("source", "history", "target"))
    path = history / filename
    performance.write_results(path, [])  # B has explicitly reset this series.
    with contextlib.redirect_stdout(io.StringIO()):
        inputs = [(old_commit, 10.0)] + [(f"{n:040x}", 100.0) for n in range(1, 10)]
        for commit, value in inputs:
            performance.write_results(source / filename, [row(commit, value)])
            assert nvx.main(["performance", "persist", "--source-dir", str(source),
                             "--history-dir", str(history)]) == 0
    performance.write_results(target / filename, [row("b" * 40, 160.0)])
    command = ["performance", "gate", "--baseline-dir", str(history),
               "--target-dir", str(target), "--window", "10",
               "--minimum-history", "10", "--threshold", "50",
               "--absolute-tolerance-ms", "10"]
    print("Fresh post-reset points: 9; expected: warmup, exit 0")
    actual = nvx.main(command)
    print("Actual exit with pre-reset row:", actual)
    performance.write_results(path, [r for r in performance.read_results(path)
                                     if r.commit != old_commit])
    print("Control: remove only the pre-reset row")
    control = nvx.main(command)
    print("Control exit:", control)
    assert control == 0
    assert actual == 0, "pre-reset persistence completed warmup with only nine new points"

The thresholds 50% / 10 ms match the current CI environment, rather than relying on the CLI defaults.

Observed reproduction output

The exact snippet published above was rerun from the pinned source snapshot and produced:

Fresh post-reset points: 9; expected: warmup, exit 0
REGRESSION: linux-kvm-virtual-machine/microvm-v2/1vcpu/openvmm_snapshot_generation: p50 160.00 ms vs 10-point base median 100.00 ms (+60.0%, +60.00 ms)
Checked 1 metric(s), found 1 regression(s) (threshold: >50% vs 10-point median; minimum history: 10; absolute latency tolerance: 10 ms).
Actual exit with pre-reset row: 1
Control: remove only the pre-reset row
WARMUP: linux-kvm-virtual-machine/microvm-v2/1vcpu/openvmm_snapshot_generation has 9 of 10 required base-branch points
Checked 0 metric(s), found 0 regression(s) (threshold: >50% vs 10-point median; minimum history: 10; absolute latency tolerance: 10 ms).
Control exit: 0
Traceback (most recent call last):
  File "<stdin>", line 44, in <module>
AssertionError: pre-reset persistence completed warmup with only nine new points
Suggested fix

Make persistence aware of resets that happened after the measurement revision. Before appending an older run's rows onto the current branch, identify reset dimensions between the run's source revision and the current target history and exclude those pre-reset rows, while retaining unaffected metrics. A persisted reset epoch/source-revision boundary per series is another option. The push-retry loop must re-evaluate this boundary after every fetch, not just its first attempt.

Add a regression test for an A-result arriving after B resets a dimension, and prove that nine post-reset rows remain 9/10 warmup. Also preserve existing per-commit idempotence and selective-reset behavior.

Checked, not filed

  • Guest-time/time-ABI parsing, chunk boundaries, report-only markers, missing/pending/failed status checks, boot/restore generations, backend rates, warp bounds, and process/benchmark caller wiring: existing targeted tests pass.
  • Guest C packet decoding, generation checks, correction thresholds, atomic state formatting, deferred-work helpers, and kernel watcher message rules: native Linux tests pass under WSL. A directory bind, rather than a single-inode bind, is used for public time state, so atomic replacement does not establish a stale-inode finding.
  • Performance-gate direction/relative and absolute thresholds, multimodal medians, insufficient histories, ordinary selective resets, CPU/ABI isolation, lifecycle instability and attempt logging: existing targeted tests pass; these do not cover the delayed-result/reset race.
  • Documentation describes a 40% / 5 ms gate while the current CI configuration passes 50% / 10 ms. Treated as low-impact documentation drift, not a separate functional finding; the confirmed reproduction uses actual CI settings.

Validation and limits

  • Existing targeted suites: 102 tests run, 99 passed, 3 skipped. Windows: 37 monitor/warp/caller tests, 15 field/profile tests, 12 gate tests, and 9 lifecycle/attempt tests. WSL/Linux: 29 guest-time C tests.
  • Skips: two tests requiring initialized pinned OpenVMM sources, and one musl comparison because musl-gcc is unavailable.
  • python scripts\nvx.py verify was run on the pinned snapshot and reported that the initialized OpenVMM submodule was absent. It was not initialized or changed for this read-only hunt.
  • Both the focused regression test and the exact snippet published above fail as expected, and removing only the stale row makes the control succeed. The Python/CSV handoff and gate behavior are reproduced; the GitHub scheduling trigger is traced in the workflow/action source, not demonstrated by modifying CI or changing real runner storage.
  • No NVX VM boot, WHP/KVM/MSHV qualification, fresh performance measurements, or live GitHub workflow rerun was attempted. No claim is made that an existing live run has already hit this race.
Lingua principale
Python
Stelle
380
Fork
24
Merge medio
12h 2m
PR unite (30g)
291

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di microsoft/nvx

Tutte le issue di microsoft/nvx

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.