Skip to content

[Tests] Fix flaky popen pool executor test - #19746

Merged
tqchen merged 1 commit into
apache:mainfrom
tlopex:fix-popen-pool-executor-flaky-timeout
Jun 12, 2026
Merged

tqchen merged 1 commit into
apache:mainfrom
tlopex:fix-popen-pool-executor-flaky-timeout

Conversation

@tlopex

@tlopex tlopex commented Jun 12, 2026

Copy link
Copy Markdown
Member

This pr increases the timeout in test_popen_pool_executor from 10ms to 15ms.

The original 10ms timeout could race with the terminate_self() case under local/full-suite load, causing the worker-exit path to be reported as TimeoutError instead of ChildProcessError.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request increases the timeout for the PopenPoolExecutor in a unit test from 10ms to 15ms. The reviewer recommends increasing this timeout further to 100ms (0.1s) to avoid potential test flakiness on busy CI runners, while still keeping the test execution time negligible.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

import tvm

pool = PopenPoolExecutor(max_workers=2, timeout=0.01)
pool = PopenPoolExecutor(max_workers=2, timeout=0.015)

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.

medium

While increasing the timeout from 10ms to 15ms reduces the likelihood of a race condition, 15ms is still extremely short and highly susceptible to flakiness in busy CI environments where CPU scheduling delays can easily exceed 15ms. Since value1 is submitted with a 100-second delay (identity_after, 1, 100), we can safely increase the pool timeout to a more robust value like 0.1 (100ms) or 0.2 (200ms). This will make the test significantly more stable on loaded CI runners while keeping the test execution time negligible.

Suggested change
pool = PopenPoolExecutor(max_workers=2, timeout=0.015)
pool = PopenPoolExecutor(max_workers=2, timeout=0.1)

@tqchen
tqchen merged commit 955c786 into apache:main Jun 12, 2026
11 checks passed
MasterJH5574 pushed a commit to MasterJH5574/tvm that referenced this pull request Jun 15, 2026
This pr increases the timeout in `test_popen_pool_executor` from 10ms to
15ms.

The original 10ms timeout could race with the `terminate_self()` case
under local/full-suite load, causing the worker-exit path to be reported
as `TimeoutError` instead of `ChildProcessError`.

(cherry picked from commit 955c786)
MasterJH5574 pushed a commit to MasterJH5574/tvm that referenced this pull request Jun 15, 2026
This pr increases the timeout in `test_popen_pool_executor` from 10ms to
15ms.

The original 10ms timeout could race with the `terminate_self()` case
under local/full-suite load, causing the worker-exit path to be reported
as `TimeoutError` instead of `ChildProcessError`.

(cherry picked from commit 955c786)
tqchen pushed a commit that referenced this pull request Oct 16, 2026
This pr increases the timeout in `test_popen_pool_executor` from 10ms to
15ms.

The original 10ms timeout could race with the `terminate_self()` case
under local/full-suite load, causing the worker-exit path to be reported
as `TimeoutError` instead of `ChildProcessError`.
tqchen pushed a commit that referenced this pull request Oct 18, 2026
This pr increases the timeout in `test_popen_pool_executor` from 10ms to
15ms.

The original 10ms timeout could race with the `terminate_self()` case
under local/full-suite load, causing the worker-exit path to be reported
as `TimeoutError` instead of `ChildProcessError`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants