Skip to content

test: Fix L0_long_running_stress - #9052

Merged
yinggeh merged 1 commit into
mainfrom
yinggeh/tri-2050-ci-l0_long_running_stress-base-failing
Nov 5, 2026
Merged

yinggeh merged 1 commit into
mainfrom
yinggeh/tri-2050-ci-l0_long_running_stress-base-failing

Conversation

@yinggeh

@yinggeh yinggeh commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

What does the PR do?

L0_long_running_stress now lowers perf_analyzer concurrency only when the measured server queue is above the 100 ms upper bound. A queue between 10 ms and 100 ms leaves concurrency unchanged. The summary report uses prettytable.HRuleStyle when that API is present, so generating the report does not emit the deprecated prettytable.ALL warning.

The same two files also clear pre-existing flake8 findings so the pre-commit check can run on this change: explicit scenario imports, import-order exceptions after sys.path setup, and comparison style.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

None.

Where should the reviewer start?

qa/L0_long_running_stress/scenarios.py queue comparison, then qa/L0_long_running_stress/stress.py report table.

Test plan:

  • pre-commit run --files qa/L0_long_running_stress/scenarios.py qa/L0_long_running_stress/stress.py

  • Run L0_long_running_stress and confirm a queue between 10 ms and 100 ms does not change concurrency, a queue above 100 ms lowers it, and the client log has no prettytable.ALL deprecation warning.

  • CI Pipeline ID:

Caveats:

No GitHub issue is linked. The flake8 edits do not change scenario behavior beyond the queue bound and the report-table API.

Background

The target queue range is (10000, 100000) microseconds. The high-side comparison used index 0, the same 10 ms value as the low-side comparison, so the 100 ms bound was unused.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

None.

The high-queue check used the 10 ms lower bound, so any queue above
10 ms lowered concurrency and the 100 ms upper bound was unused.
Compare against the upper bound. Build the report with HRuleStyle
when prettytable provides it, and print client.log once when the
stress process fails.

Keep test_util as the only import below sys.path, since it is the
only module loaded from qa/common.

Signed-off-by: Yingge He <yinggeh@nvidia.com>
@yinggeh yinggeh self-assigned this Nov 2, 2026
@greptile-apps

greptile-apps Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR fixes the queue threshold used by L0_long_running_stress.

  • The stress test lowers concurrency only when the server queue exceeds 100 ms.
  • The stress report uses the current PrettyTable style API when available.
  • Failed stress runs print the client log once and then show the final status.
  • The stress-test files use clearer imports and comparison styles without changing test behavior.

No actionable issues were found. The full stress test was not run.

Reviews (1) · Last reviewed commit: "test: Hold stress concurrency inside the..." · Reviewed by Greptile

@whoisj whoisj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Vinya567
Vinya567 self-requested a review November 2, 2026 22:55
@yinggeh
yinggeh merged commit 25419f9 into main Nov 5, 2026
7 checks passed
@yinggeh
yinggeh deleted the yinggeh/tri-2050-ci-l0_long_running_stress-base-failing branch November 5, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants