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

WIP - pkg/flock package #378

Closed
wants to merge 1 commit into from
Closed

Conversation

vrothberg
Copy link
Member

Introduce the pkg/flock package, a flock(2)-based implementation in
golang that can be used within and across process spaces. The main
motivation for this package is to benefit from the TryLock-semantics
offered by flock(2), which is not possible with c/storage.Locker file
lock since they are based on fcntl(2).

Signed-off-by: Valentin Rothberg [email protected]

pkg/flock/flock_windows.go Outdated Show resolved Hide resolved
pkg/flock/flock_unix.go Outdated Show resolved Hide resolved
// synchronized by the caller.
func (f *flock) open() error {
if f.fd != -1 || f.flockType != flockUnlocked {
return fmt.Errorf("%v: trying to open improper Flock %q %d %d", errInternal, f.path, f.fd, f.flockType)
Copy link
Member

Choose a reason for hiding this comment

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

please use errors.Wrapf

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially I had that but the problem is that Wrapf mixes the order. I want to have a "internal error ..." but using Wrapf would move that to the end.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps errors.WithStack()?

Introduce the pkg/flock package, a flock(2)-based implementation in
golang that can be used within and across process spaces. The main
motivation for this package is to benefit from the TryLock-semantics
offered by flock(2), which is not possible with c/storage.Locker file
lock since they are based on fcntl(2).

Signed-off-by: Valentin Rothberg <[email protected]>
@vrothberg vrothberg changed the title pkg/flock package WIP - pkg/flock package Jul 1, 2019
@vrothberg
Copy link
Member Author

Marking it as WIP until there's a user of this API, presumably the copy-blob-detection in c/image.

// New returns a Flock for path.
func New(path string) (flock, error) {
// Make sure we can operate on the path.
fd, err := unix.Open(path, os.O_RDWR|os.O_CREATE, unix.S_IRUSR|unix.S_IWUSR)
Copy link
Member

Choose a reason for hiding this comment

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

Should we use an unix.Access check for this rather then opening and closing? Would that be faster? And then only create the file if it does not exists.

@rhatdan
Copy link
Member

rhatdan commented Jul 1, 2019

@nalind WDYT?

@@ -0,0 +1,63 @@
// flock exposes an API around flock(2) but allows for synchronizing within and
// across process spaces, making it suitable for currently running threads and
Copy link
Member Author

Choose a reason for hiding this comment

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

s/currently/concurrently/

@rhatdan
Copy link
Member

rhatdan commented Jul 1, 2019

@vrothberg Makes sense, since I envisioned @nalind Blocking this if it was not used in containers/storage.

@rhatdan
Copy link
Member

rhatdan commented Jul 10, 2019

@vrothberg Lets talk about this in the next meeting.

@vrothberg
Copy link
Member Author

Ping. @rhatdan, let's check if we still/really need this PR.

@rhatdan
Copy link
Member

rhatdan commented Aug 23, 2019

@vrothberg Needs a rebase. New conversations with @saschagrunert might get us a user for this, yet.

@vrothberg
Copy link
Member Author

@rhatdan, I prefer to wait until we're certain there will be a user.

@rhatdan
Copy link
Member

rhatdan commented Nov 21, 2022

@vrothberg Since this has been around for 2+ years, can we close it?

@vrothberg
Copy link
Member Author

Actually over 3 years. Time flies by!

@vrothberg vrothberg closed this Nov 22, 2022
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