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

[Bugfix] Revert "[Bugfix] use SoftLockFile instead of LockFile (#3578)" #3599

Merged
merged 3 commits into from
Mar 25, 2024

Conversation

WoosukKwon
Copy link
Collaborator

Reverts #3578 for backward compatibility.

@kota-iizuka
Copy link
Contributor

@WoosukKwon Please use lockfile.Lockfile(mode=666) to fix bugs with multi-user usage. #3578 (comment)

@youkaichao
Copy link
Member

Let me send a fix.

@youkaichao
Copy link
Member

@kota-iizuka can you test if this branch works as you want now?

@youkaichao youkaichao requested a review from zhuohan123 March 25, 2024 01:27
@kota-iizuka
Copy link
Contributor

can you test if this branch works as you want now?

@youkaichao I did an experiment, but contrary to expectations I got the same permission denied error. I'll look into the method a little more

@kota-iizuka
Copy link
Contributor

I'll look into the method a little more

This modification in vLLM is OK for me.

I found the bug is in filelock. https://github.com/tox-dev/filelock/blob/main/src/filelock/_unix.py#L38 uses os.O_CREAT, so if a file already created by another user exists, It will be the permission error.

@youkaichao
Copy link
Member

If multiple users use one server together, they should have their own directory for storing models.

This modification in vLLM is OK for me.

Then I will merge it now.

@youkaichao youkaichao merged commit 56a8652 into main Mar 25, 2024
32 checks passed
@youkaichao youkaichao deleted the revert-softlock branch March 25, 2024 03:06
Temirulan pushed a commit to Temirulan/vllm-whisper that referenced this pull request Sep 6, 2024
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.

4 participants