Skip to content

test: accept exact queue-delay boundary (>=) in seq-batcher timing as… - #8899

Merged
mc-nv merged 5 commits into
mainfrom
asaraswathi/tri-1372-fix-ci-test-l0_sequence_batcher-rtx50-rhel
Aug 5, 2026
Merged

test: accept exact queue-delay boundary (>=) in seq-batcher timing as…#8899
mc-nv merged 5 commits into
mainfrom
asaraswathi/tri-1372-fix-ci-test-l0_sequence_batcher-rtx50-rhel

Conversation

@akhilraj9

Copy link
Copy Markdown
Contributor

What does the PR do?

Fix a strict-inequality timing boundary in the sequence batcher test helper that
caused test_queue_delay_half_min_util and test_queue_delay_full_min_util to
fail deterministically on the RTX50/RHEL runner. Changes > to >= at 4 sites
in qa/common/sequence_util.py so that a response arriving in exactly the
configured queue delay (3000 ms) is correctly accepted.

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:

  • test

Related PRs:

None

Where should the reviewer start?

qa/common/sequence_util.py — 4 one-line changes at lines 677, 710, 886, 1103.
Each changes (...) > gt_ms to (...) >= gt_ms. No logic change beyond the
boundary condition. All other files are untouched.

Test plan:

  1. Run L0_sequence_batcher--RTX50--rhel and L0_sequence_batcher_shm--RTX50--rhel
    on the RTX50 RHEL runner with TRITON_RHEL=1.
  2. Confirm test_queue_delay_half_min_util and test_queue_delay_full_min_util
    pass (num_failures=0, num_errors=0).
  3. Confirm with repetition if this test passes on multiple runs — to check for flakiness.
  • CI Pipeline ID: 59190261

Caveats:

Verification depends on TRI-1608 (gRPC/protobuf build fix) being merged to main
across common, core, and third_party repos.

Background

The server correctly waits exactly max_queue_delay_microseconds (3000 ms) before
dispatching when minimum_slot_utilization cannot be met. Time is measured as
int(round(time.time()*1000)) — whole milliseconds. On the low-jitter RTX50/RHEL
runner, elapsed time rounds to exactly 3000 ms, so 3000 > 3000 → False → AssertionError. On slower machines, OS jitter adds ≥ 0.5 ms and it rounds to
3001+ → passes. The strict > has been in the code since 2019 and was newly
exposed by the faster RTX50 hardware. >= is correct — it asserts "waited at
least
the configured delay" while still catching too-early responses.


Log evidence — BEFORE fix (4 consecutive nightly failures, shm job = TRI-1379):

The exact assertion error, identical across all 4 failing runs:

raw_2.txt (2026-05-16) — L0_sequence_batcher_shm--RTX50--rhel
line 11669: *** Test test_queue_delay_half_min_util Failed
line 11757: *** Test test_queue_delay_full_min_util Failed

raw_3.txt (2026-05-16) — L0_sequence_batcher_shm--RTX50--rhel
line 11669: *** Test test_queue_delay_half_min_util Failed
line 11757: *** Test test_queue_delay_full_min_util Failed

raw_5.txt (2026-05-13) — L0_sequence_batcher_shm--RTX50--rhel
line 11717: *** Test test_queue_delay_half_min_util Failed
line 11812: *** Test test_queue_delay_full_min_util Failed

raw_6.txt (2026-07-16) — L0_sequence_batcher_shm--RTX50--rhel
line 12230: *** Test test_queue_delay_half_min_util Failed
line 12308: *** Test test_queue_delay_full_min_util Failed
The measured value was exactly 3000 ms against a threshold of 3000 ms in every
single run — deterministic boundary hit, not random flakiness.


Log evidence — AFTER fix (pipeline #59190261, 2026-07-23):

raw_8.txt — CI_JOB_NAME=L0_sequence_batcher--RTX50--rhel (TRI-1372)
line 12043: python3 sequence_batcher_test.py SequenceBatcherTest.test_queue_delay_half_min_util
line 12059: num_failures=0 num_errors=0 num_tests=1 ✅
line 12153: python3 sequence_batcher_test.py SequenceBatcherTest.test_queue_delay_full_min_util
line 12167: num_failures=0 num_errors=0 num_tests=1 ✅
line 12403: *** Test Passed ***

raw_9.txt — CI_JOB_NAME=L0_sequence_batcher_shm--RTX50--rhel (TRI-1379)
line 12332: python3 sequence_batcher_test.py SequenceBatcherTest.test_queue_delay_half_min_util
num_failures=0 num_errors=0 num_tests=1 ✅
line 12442: python3 sequence_batcher_test.py SequenceBatcherTest.test_queue_delay_full_min_util
num_failures=0 num_errors=0 num_tests=1 ✅
line 12606: *** Test Passed ***


Related Issues:

  • Closes TRI-1372
  • Closes TRI-1379

@akhilraj9 akhilraj9 self-assigned this Jul 23, 2026
@akhilraj9 akhilraj9 added the bug Something isn't working label Jul 23, 2026
@pskiran1 pskiran1 added PR: test Adding missing tests or correcting existing test and removed bug Something isn't working labels Jul 23, 2026
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a strict-inequality timing boundary in qa/common/sequence_util.py that caused test_queue_delay_half_min_util and test_queue_delay_full_min_util to fail deterministically on the low-jitter RTX50/RHEL runner, where the measured elapsed time rounds to exactly the configured queue delay (3000 ms).

  • Four assertion sites change > to >= and update the failure message from "expected greater than" to "expected greater than or equal to", keeping operator and message in sync across all sites.
  • Minor housekeeping: the unnecessary global _deferred_exceptions declaration (list mutation does not require global in Python) is removed, and a # flake8: noqa: F403,F405 comment is added to silence star-import warnings inherited from the existing QA suite convention.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-understood boundary fix in test helper code with no production impact.

All four assertion sites are updated consistently: the operator changes from > to >= and the failure message is updated to match at every site. The fix correctly reflects the server's documented behavior of waiting exactly max_queue_delay_microseconds before dispatching. The removal of the unnecessary global declaration and addition of the flake8 suppression comment are both correct. CI evidence shows the previously-deterministic failures are eliminated.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
qa/common/sequence_util.py Four timing assertion sites updated from strict > to >=; failure messages updated to match; unnecessary global keyword removed; flake8 noqa comment added — all changes are correct and consistent.

Sequence Diagram

sequenceDiagram
    participant T as Test Runner
    participant S as Triton Server
    participant A as Assertion (sequence_util.py)

    T->>S: Send inference sequence (start flag)
    Note over T: seq_start_ms = now()
    S-->>T: Response(s)
    T->>S: Send inference (end flag)
    S-->>T: Final response
    Note over T: seq_end_ms = now()
    T->>A: "elapsed = seq_end_ms - seq_start_ms"
    alt "elapsed >= gt_ms (queue delay threshold)"
        A-->>T: PASS
    else "elapsed < gt_ms"
        A-->>T: FAIL — response arrived too early
    end
Loading

Reviews (5): Last reviewed commit: "Merge branch 'main' into asaraswathi/tri..." | Re-trigger Greptile

@pskiran1

Copy link
Copy Markdown
Member

@akhilraj9, please update the copyright to 2019-2026.

pskiran1
pskiran1 previously approved these changes Jul 24, 2026
# other QA files. Silence flake8's star-import warnings (F403/F405) for this
# file only; proper cleanup is tracked separately.
# flake8: noqa: F403,F405

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Scoped # flake8: noqa: F403,F405 for the QA-suite import * convention. Note it also masks two pre-existing in0dummy_in0 typos in the string-tensor path — tracked for a proper fix in TRI-1643.

@Vinya567 Vinya567 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

@mc-nv
mc-nv merged commit be7429f into main Aug 5, 2026
4 checks passed
@mc-nv
mc-nv deleted the asaraswathi/tri-1372-fix-ci-test-l0_sequence_batcher-rtx50-rhel branch August 5, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: test Adding missing tests or correcting existing test

Development

Successfully merging this pull request may close these issues.

5 participants