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

xattrs: use unix flocks when writing multiple xattrs #2528

Merged
merged 9 commits into from
Feb 18, 2022

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Feb 10, 2022

checking the flock implementation options...

@dragotin
Copy link
Contributor

I think this can only be the start.

A bit of copy & paste from the flock() manpage (linux system call):

  • flock() does not lock files over NFS.
  • flock() places advisory locks only; given suitable permissions on a file, a process is free to ignore the use of flock() and perform I/O on the file.
  • flock() is blocking. I would have implemented a lock base function that uses the unblocking variant of flock, but waits for a few milliseconds and tries again.
  • The read function All() (at least) needs to place a shared lock for read. That must be considered here as well.
  • Any reason why you did not choose a flock golang library that is multi platform?

@butonic
Copy link
Contributor Author

butonic commented Feb 11, 2022

https://github.com/gofrs/flock was the first implementation we looked at, but stumbled over the fact that it is intendet to write a lock file, not folders. Furthermore, we discovered the proposal to expose the existing locking implementation in the internal golang/go/src/cmd/go/internal/lockedfile package. But all threads lead to file locks, so I implemented a unix based lock.

@butonic
Copy link
Contributor Author

butonic commented Feb 11, 2022

we will use https://github.com/rogpeppe/go-internal/blob/master/lockedfile/lockedfile.go https://github.com/gofrs/flock to write a {node}.flock file when setting multiple extended attributes, OR when creating a application level log ({node}.lock).

it comes with nice TryLockCtx methods which wrap around the functionality we want.

butonic and others added 6 commits February 17, 2022 19:14
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
To lock, a file with the ending .flock is created beside the to
be locked node (which also can be a directory) which is locked
with a Lock. Other ocis processes respect that.
Co-authored-by: David Christofas <dchristofas@owncloud.com>
@butonic butonic marked this pull request as ready for review February 17, 2022 21:12
Copy link
Contributor

@dragotin dragotin left a comment

Choose a reason for hiding this comment

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

Awesome stuff!

Co-authored-by: David Christofas <dchristofas@posteo.de>
@butonic butonic merged commit 6901ae6 into cs3org:edge Feb 18, 2022
@butonic butonic deleted the flock_xattr_set_multiple branch February 18, 2022 10:08
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.

3 participants