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.

Sign up for GitHub

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

Document instance naming on reuse #6018

Closed
wants to merge 5 commits into from
Closed

Document instance naming on reuse #6018

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
45ce9d8
Update and refactor test for adding name into instance signature
kthui Jul 3, 2023
28efec6
Revert "Update and refactor test for adding name into instance signat…
kthui Jul 12, 2023
172ddf2
Document may reuse an instance differ in name
kthui Jul 13, 2023
3e2cb78
Update docs wording
kthui Jul 13, 2023
9299f2a
Fix documentation
kthui Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/user_guide/model_management.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ request is received under
'config.pbtxt' is modified in place. The swap file is not part of the model
configuration, so its presence in the model directory may be detected as a new file
and cause the model to fully reload when only an update is expected.
* When a model configuration is reloaded existing model instances will be
checked against the new configuration and be reused if possible. If a model
instance is reused it will retain its original name.
Comment on lines +227 to +229
Copy link
Contributor

@rmccorm4 rmccorm4 Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Moving comments so far to a thread.

Copy link
Contributor

@rmccorm4 rmccorm4 Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Personally, I think this is not important to document right now and may be confusing or changed in the near future. If we're planning some reworks or reconsiderations of naming behavior as discussed in core PR, I think we can just close this PR and leave out this comment for now and update the documentation when we make some behavioral changes.

What do you think @GuanLuo @kthui ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Use hyperlink... I am leaning towards that this documentation is not necessary as well: what additional information that the user will gain from reading it? When the instance is "reused" (I am not sure about the use of this word as well), is the reader expecting the instance name will be different and thus we now need to state that the name is the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree the documentation does not help the user reading it. I will close this PR.


* If a sequence model is updated with in-flight sequence(s), Triton does not
guarentee any remaining request(s) from the in-flight sequence(s) will be routed
Expand Down