Skip to content

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

Merged
youkaichao merged 3 commits intomainfrom
revert-softlock
Mar 25, 2024
Merged

[Bugfix] Revert "[Bugfix] use SoftLockFile instead of LockFile (#3578)"#3599
youkaichao merged 3 commits intomainfrom
revert-softlock

Conversation

@WoosukKwon
Copy link
Copy Markdown
Collaborator

Reverts #3578 for backward compatibility.

@kota-iizuka
Copy link
Copy Markdown
Contributor

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

@youkaichao
Copy link
Copy Markdown
Member

Let me send a fix.

@youkaichao
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

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