Skip to content

[CI] Fix custome offline ci issue, V1 Others Test bug#34913

Closed
yewentao256 wants to merge 2 commits intomainfrom
wentao-fix-ci-custom-logits-issue
Closed

[CI] Fix custome offline ci issue, V1 Others Test bug#34913
yewentao256 wants to merge 2 commits intomainfrom
wentao-fix-ci-custom-logits-issue

Conversation

@yewentao256
Copy link
Member

@yewentao256 yewentao256 commented Feb 19, 2026

Purpose

Fixes https://buildkite.com/vllm/ci/builds/52111/steps/canvas?jid=019c71de-e90f-46a7-9da5-cfea9f4a0660

We need to fork, but if we let vLLM automatically choose attn backend,

    if cuda_is_initialized():
        reasons.append("CUDA is initialized")
    ...
    if reasons:
        logger.warning(
            "We must use the `spawn` multiprocessing start method. "
            "Overriding VLLM_WORKER_MULTIPROC_METHOD to 'spawn'. "
            ...
        )
        os.environ["VLLM_WORKER_MULTIPROC_METHOD"] = "spawn"

It will be set to spawn, which causes the issue

Test

[yewentao256@nma-frk-sa-h100-00-preserve vllm-source]$ pytest tests/v1/logits_processors/test_custom_offline.py::test_custom_logitsprocs[CustomLogitprocSource.LOGITPROC_SOURCE_ENTRYPOINT]
================================================ test session starts =================================================
platform linux -- Python 3.12.9, pytest-9.0.2, pluggy-1.6.0
rootdir: /home/yewentao256/vllm-source
configfile: pyproject.toml
plugins: anyio-4.12.0, asyncio-1.3.0
asyncio: mode=Mode.STRICT, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
collected 1 item                                                                                                     

tests/v1/logits_processors/test_custom_offline.py .                                                            [100%]

================================================== warnings summary ==================================================
<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type SwigPyPacked has no __module__ attribute

<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type SwigPyObject has no __module__ attribute

tests/v1/logits_processors/test_custom_offline.py::test_custom_logitsprocs[CustomLogitprocSource.LOGITPROC_SOURCE_ENTRYPOINT]
  /home/yewentao256/vllm-source/tests/utils.py:1002: DeprecationWarning: This process (pid=366690) is multi-threaded, use of fork() may lead to deadlocks in the child.
    pid = os.fork()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================================== 1 passed, 3 warnings in 36.55s ===========================================

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@yewentao256 yewentao256 added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 19, 2026
@mergify mergify bot added v1 bug Something isn't working labels Feb 19, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

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 addresses a CI failure in the custom offline logits processor tests. The issue stems from vLLM's automatic attention backend selection triggering CUDA initialization, which then forces the multiprocessing start method to 'spawn', conflicting with the test's requirement for 'fork'. The fix cleverly circumvents this by explicitly setting the attention backend to 'TRITON_ATTN'. Additionally, the PR commendably improves the test's correctness by propagating extra keyword arguments to the reference LLM instance, ensuring a more accurate comparison. The changes are sound and effective. I have one suggestion to further improve the code's maintainability by removing a magic string.

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@MatthewBonanni
Copy link
Collaborator

This will be fixed by #34818

@yewentao256
Copy link
Member Author

@MatthewBonanni Thanks, I am thinking we can still have this PR just in case we might have similar issue in the future, fork is a must-have for this test, what's your thought?

@MatthewBonanni
Copy link
Collaborator

MatthewBonanni commented Feb 19, 2026

34818 allows this test to run with fork. If you think this PR can land first and get the test green I think it's fine but if so, I'd probably revert this change as part of 34818.

I feel that this PR introduces a false pass. As it exists on main, the test correctly fails and catches a bug (even though it's not necessarily what the test was designed to catch) - this PR prevents it from failing and catching the bug

@yewentao256
Copy link
Member Author

@MatthewBonanni OK, please combine this diff in your PR.
Actually I think we need this, as this is not expected to catch that spwan/fork bug (We might have new unit tests specially for that test)

@yewentao256 yewentao256 deleted the wentao-fix-ci-custom-logits-issue branch February 19, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants