feat(slayerfs): impl plock for slayerfs #339
Conversation
Signed-off-by: zine yu <zine.xlws@gmail.com>
Signed-off-by: zine yu <zine.xlws@gmail.com>
Signed-off-by: zine yu <zine.xlws@gmail.com>
Signed-off-by: zine yu <zine.xlws@gmail.com>
Signed-off-by: zine yu <zine.xlws@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR implements POSIX file lock (plock) functionality for SlayerFS, adding support for byte-range locking across three metadata storage backends (Redis, etcd, and Database/SQLite/PostgreSQL).
Key Changes
- Adds new
file_lockmodule withFileLockType,PlockRecord,FileLockRange, and related types - Implements plock operations (
get_plock,set_plock) in all three metadata stores - Integrates file locking into VFS layer and FUSE adapter via
getlkandsetlkoperations - Adds comprehensive test coverage for lock scenarios (read/write locks, conflicts, releases, cross-session visibility)
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
project/slayerfs/src/meta/file_lock.rs |
Core file lock types and logic including conflict detection and lock merging |
project/slayerfs/src/meta/stores/redis_store.rs |
Redis-backed plock implementation with session-based lock storage |
project/slayerfs/src/meta/stores/etcd_store.rs |
etcd-backed plock implementation with atomic update support |
project/slayerfs/src/meta/stores/database_store.rs |
Database (SQLite/PostgreSQL) plock implementation with transactional guarantees |
project/slayerfs/src/fuse/mod.rs |
FUSE integration for getlk and setlk operations |
project/slayerfs/src/vfs/fs.rs |
VFS layer plock methods wrapping MetaLayer calls |
project/slayerfs/src/meta/entities/plock_meta.rs |
Database entity definition for plock table |
project/slayerfs/tests/scripts/test_meta_store.sh |
Test automation script for metadata store testing |
project/slayerfs/doc/file_lock_testing_guide.md |
Comprehensive testing guide and documentation |
buckal.snap |
Build snapshot with dependency updates (contains merge conflict) |
project/slayerfs/Cargo.toml |
Dependency updates for rfuse3 with file-lock feature |
project/slayerfs/BUCK |
Build configuration updates for local rfuse3 dependency |
Signed-off-by: zine yu <zine.xlws@gmail.com>
Ivanbeethoven
left a comment
Ivanbeethoven
left a comment
There was a problem hiding this comment.
LGTM, fix lock release later
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Err(e) => Err(match e { | ||
| MetaError::NotFound(_) => libc::ENOENT, | ||
| MetaError::AlreadyExists { .. } => libc::EAGAIN, // Lock conflict | ||
| MetaError::NotSupported(_) => libc::ENOSYS, | ||
| _ => libc::EIO, |
There was a problem hiding this comment.
When FUSE setlk receives a lock conflict, the metadata backends return MetaError::LockConflict, but the match in setlk only translates MetaError::AlreadyExists to EAGAIN; all other errors fall through to EIO. This means any real lock contention will surface to the kernel as an I/O error instead of a retryable EAGAIN, breaking POSIX lock semantics whenever a lock is already held. Handle MetaError::LockConflict the same way as AlreadyExists so callers see a busy lock rather than an I/O failure.
Useful? React with 👍 / 👎.
| // Query specific owner and session | ||
| let row = PlockMeta::find() | ||
| .filter(plock_meta::Column::Inode.eq(inode)) | ||
| .filter(plock_meta::Column::Owner.eq(query.owner)) | ||
| .filter(plock_meta::Column::Sid.eq(*sid)) |
There was a problem hiding this comment.
The database implementation of get_plock filters the lookup by both owner and the current session id, so it only inspects locks owned by the querying process in the same session. If another owner/session holds an overlapping lock, this code returns an unlocked state rather than the conflicting lock, unlike the etcd/redis backends. As a result F_GETLK on the DB backend reports the region as free even though it is locked by someone else. Drop the owner/sid filters or merge all records so conflicts from other sessions are visible.
Useful? React with 👍 / 👎.
|
@codex Why the GitHub Action hang for waiting? |
|
Codex couldn't complete this request. Try again later. |
|
@claude Review the PR |
Signed-off-by: zine yu <zine.xlws@gmail.com>
3 participants
No description provided.