Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix L0_implicit_state and it's variants #7940

Closed
wants to merge 3,500 commits into from

Conversation

indrajit96
Copy link
Contributor

@indrajit96 indrajit96 commented Jan 15, 2025

What does the PR do?

Update the config file with max_sequence_idle_microseconds to avoid sequence timeouts in server

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:

NA

Where should the reviewer start?

Config file

Test plan:

NA

Caveats:

Background

Server waits for max_sequence_idle_microseconds before closing the request sequence and expects the client to start a NEW sequence for ANY subsequent request.
Test was failing because the max_sequence_idle_microseconds was set to low causing the server to believe the sequence is over and reject any further requests with SAME sequence_id without the START flag. Causing the test to fail.

yinggeh and others added 30 commits February 27, 2024 12:00
* Modify "header_forward_pattern" to match headers case-insensitively. Add unit tests.

* fix indentation

* fix pre-comiit errors

* Update doc

* Update copyright

* Add test case for "(?-i)", which disables regex case-insensitive mode.

* fix pre-commit

* Name each test. Remove support of disabling --http-header-forward-pattern case-insensitive mode on http python client.

* Update .md file.

* fix typo

* Reformat args.

* Fix pre-commit

* Fix test name issue.

* Fix pre-commit.

* Update md file and copyright.
* Update README and versions for 2.43.0 / 24.02

* Update Dockefile to reduce image size.

* Update path in patch file for model generation

Update README.md post-24.02
* patching git repository parameterization from production branch 1

* Fix go package directory name

* pre-commit fixes

* pre-commit fixes

---------

Co-authored-by: kyle <[email protected]>
* Enhance bound check for shm offset

* Add test for enhance bound check for shm offset

* Fix off by 1 on max offset

* Improve comments

* Improve comment and offset

* Separate logic between computation and validation
…6017)

* Allow non-decoupled model to send response and FINAL flag separately

* Update copyright

* Defer sending error until FINAL flag is seen to avoid invalid reference

* Move timestamp capture location

* Delay time-point of response complete timestamp in GPRC and SageMaker endpoint

* Move location of RESPONSE_COMPLETE timestamp capture to better align with the meaning.
Added a test case to check for optional/required input params in a request and appropriate response from server.
Includes addition of 3 simple models with a combination of required/optional input params
Add flag to enable compile of OpenAI support in PA
* Test Correlation Id string support for BLS
* Add AsyncIO HTTP compression test

* Improve command line option handling
* Update Docerkfile to install genai

* Change the installation script

* install both build and hatch

* Update name

---------

Co-authored-by: Elias Bermudez <[email protected]>
* Added TRITONSERVER_InferenceTraceSetContext logic
…odes (#6992)

* Add documentation for mapping between Triton Errors and HTTP status codes

* formatting

* Update README.md
* Update README and versions for 2.44.0 / 24.03 (#6971)

* Update README and versions for 2.44.0 / 24.03

* Mchornyi 24.03 (#6972)

* Current location is dropped in 12.4

* Update Dockerfile.win10.min

* Change to triton_sample_folder (#6973)

---------

Co-authored-by: kyle <[email protected]>
Co-authored-by: Misha Chornyi <[email protected]>

* Specify path for PyTorch model extension library (#7025)

* Update README.md 2.44.0 / 24.03 (#7032)

* Update README.md post-24.03

---------

Co-authored-by: Kyle McGill <[email protected]>
Co-authored-by: kyle <[email protected]>
* Fix Otel version

* Fix version in CPU metrics

* Update metrics.md

* Update trace.md
* Add testing for iterative scheduler backlogged requests

* Update test count
* Remove conda from build

* Escape slash symbol

* Escape slash: align

* Escape slash: align

* Escape slash: align

* Escape slash: align

* Install virtualenv

* Fix vLLM flag

* remove conda flag

* Fix code style
Copy link
Contributor

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

LGTM, just copyright nits, thanks for the fix!

Also, please expand on the bug and the fix more in the description to educate future readers.

Ex: if sequence times out mid sequence, it will interpret a request as the start of a new sequence, but client won't have set the START flag because it thinks it's still in the middle of a sequence, so an error will be raised -- but in nicer words

@@ -1,5 +1,5 @@
#!/bin/bash
# Copyright 2021-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# Copyright 2021-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't update copyright if file isn't modified

@@ -1,4 +1,4 @@
# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright 2023-2025

@pvijayakrish pvijayakrish force-pushed the ibhosale_L0_implicit_state branch from 0cd8a05 to a0d1b22 Compare January 15, 2025 17:13
@yinggeh
Copy link
Contributor

yinggeh commented Jan 15, 2025

Please clean up the commit history.

@rmccorm4
Copy link
Contributor

The original change was so small, I'd probably just make a fresh branch for simplicity. I think that force push from Pavithra messed up the history.

@indrajit96
Copy link
Contributor Author

The original change was so small, I'd probably just make a fresh branch for simplicity. I think that force push from Pavithra messed up the history.

Yup :)

@indrajit96 indrajit96 closed this Jan 16, 2025
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.