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

Evict requests if the client has disconnected #165

Open
aniketmaurya opened this issue Jul 8, 2024 · 4 comments · May be fixed by #223
Open

Evict requests if the client has disconnected #165

aniketmaurya opened this issue Jul 8, 2024 · 4 comments · May be fixed by #223
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@aniketmaurya
Copy link
Collaborator

🚀 Feature

If a client sends a request and is disconnected, LitServe still processes the request which is a waste of computation. We need to evict the request if the client is disconnected.

Motivation

Pitch

Alternatives

Additional context

@aniketmaurya aniketmaurya added enhancement New feature or request help wanted Extra attention is needed labels Jul 8, 2024
@bhimrazy
Copy link
Contributor

Hi @aniketmaurya, I've been exploring potential implementations for this feature
and came across the req.is_disconnected() method, which seems useful for checking if the client has disconnected.
I will try to draft a PR within the next few days to address this issue.

Looking forward to your thoughts on it! 😊

@aniketmaurya
Copy link
Collaborator Author

aniketmaurya commented Aug 12, 2024

sounds great @bhimrazy!! the only thing to take care of would be that we send the request to another process so request might lose the ability to check if it's disconnected (haven't tested personally). But definitely go ahead and give it a try.

Another idea could be is to check using socket connection but we can try that later.

@bhimrazy
Copy link
Contributor

Thank you, @aniketmaurya, for the insights, and apologies for the delay in drafting the PR.
I've tried implementing an idea by checking for the uid. However, I'm a bit unsure if this will impact performance.
It's still a work in progress, but I would appreciate any feedback.

@aniketmaurya
Copy link
Collaborator Author

Commented on the PR @bhimrazy! let's move the conversation there.

@bhimrazy bhimrazy linked a pull request Aug 26, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
2 participants