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.

Sign up for GitHub

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

Jump to bottom

Support no timeout locks on db files #35

Merged
merged 2 commits into from
Sep 6, 2017
Merged

Support no timeout locks on db files #35

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2a6484
Added support for no timeout locks on db files
dmoklaf Sep 5, 2017
f1ea5d8
Added proposed changes
dmoklaf Sep 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 19 additions & 15 deletions bolt_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,34 @@ import (

// flock acquires an advisory lock on a file descriptor.
func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) error {
maxSleep := timeout - timeoutStep
var t time.Time
if maxSleep >= 0 {
t = time.Now()
}
fd := db.file.Fd()
flag := syscall.LOCK_NB
if exclusive {
flag |= syscall.LOCK_EX
} else {
flag |= syscall.LOCK_SH
}
for {
// If we're beyond our timeout then return an error.
// This can only occur after we've attempted a flock once.
if t.IsZero() {
t = time.Now()
} else if timeout > 0 && time.Since(t) > timeout {
return ErrTimeout
}
flag := syscall.LOCK_SH
if exclusive {
flag = syscall.LOCK_EX
}

// Otherwise attempt to obtain an exclusive lock.
err := syscall.Flock(int(db.file.Fd()), flag|syscall.LOCK_NB)
// Attempt to obtain an exclusive lock.
err := syscall.Flock(int(fd), flag)
if err == nil {
return nil
} else if err != syscall.EWOULDBLOCK {
return err
}

// If we timed out then return an error.
if timeout != 0 && (maxSleep < 0 || time.Since(t) > maxSleep) {
return ErrTimeout
}

// Wait for a bit and try again.
time.Sleep(50 * time.Millisecond)
time.Sleep(timeoutStep)
}
}

Expand Down
40 changes: 20 additions & 20 deletions bolt_unix_solaris.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,35 @@ import (

// flock acquires an advisory lock on a file descriptor.
func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) error {
maxSleep := timeout - timeoutStep
var t time.Time
if maxSleep >= 0 {
t = time.Now()
}
fd := db.file.Fd()
var lockType int16
if exclusive {
lockType = syscall.F_WRLCK
} else {
lockType = syscall.F_RDLCK
}
for {
// If we're beyond our timeout then return an error.
// This can only occur after we've attempted a flock once.
if t.IsZero() {
t = time.Now()
} else if timeout > 0 && time.Since(t) > timeout {
return ErrTimeout
}
var lock syscall.Flock_t
lock.Start = 0
lock.Len = 0
lock.Pid = 0
lock.Whence = 0
lock.Pid = 0
if exclusive {
lock.Type = syscall.F_WRLCK
} else {
lock.Type = syscall.F_RDLCK
}
err := syscall.FcntlFlock(db.file.Fd(), syscall.F_SETLK, &lock)
// Attempt to obtain an exclusive lock.
lock := syscall.Flock_t{Type: lockType}
err := syscall.FcntlFlock(fd, syscall.F_SETLK, &lock)
if err == nil {
return nil
} else if err != syscall.EAGAIN {
return err
}

// If we timed out then return an error.
if timeout != 0 && (maxSleep < 0 || time.Since(t) > maxSleep) {
Copy link
Contributor

@heyitsanthony heyitsanthony Sep 5, 2017 •
edited

Choose a reason for hiding this comment

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

can get rid of maxSleep:

func flock(...) error {
    t := time.Now()
    fd := db.file.Fd()
...

         if timeout != 0 && time.Since(t) > (timeout - flockRetryTimeout) {
             return ErrTimeout
         }
         time.Sleep(flockRetryTimeout)
    }

Copy link
Contributor Author

@dmoklaf dmoklaf Sep 6, 2017 •
edited

Choose a reason for hiding this comment

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

The goal of the initial pull request was specifically to allow the API user to not depend upon any time logic - return directly with an error if the database file cannot be locked, without depending on the OS time settings.

That proposed code change would specifically create a hidden dependency with the time module. I checked it for correctness, it would work under Go 1.9 which brings monotonic guarantees. However under previous Go version, there are no monotonic guarantees and the code could result in a time.Sleep call (e.g., if the OS time is changed for whatever reason).

So it boils down to which version of Go do we support to guarantee that boltdb work. Do we want this patch to work for Go versions before 1.9 too, or do we design only for the latest version of Go? In the latter case, fine for me, I can push an update.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think relying on >=1.9 is OK since this is a new feature that won't interfere with old behavior.

return ErrTimeout
}

// Wait for a bit and try again.
time.Sleep(50 * time.Millisecond)
time.Sleep(timeoutStep)
}
}

Expand Down
32 changes: 17 additions & 15 deletions bolt_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,30 +58,32 @@ func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) erro
}
db.lockfile = f

maxSleep := timeout - timeoutStep
var t time.Time
if maxSleep >= 0 {
t = time.Now()
}
fd := db.file.Fd()
var flag uint32 = flagLockFailImmediately
if exclusive {
flag |= flagLockExclusive
}
for {
// If we're beyond our timeout then return an error.
// This can only occur after we've attempted a flock once.
if t.IsZero() {
t = time.Now()
} else if timeout > 0 && time.Since(t) > timeout {
return ErrTimeout
}

var flag uint32 = flagLockFailImmediately
if exclusive {
flag |= flagLockExclusive
}

err := lockFileEx(syscall.Handle(db.lockfile.Fd()), flag, 0, 1, 0, &syscall.Overlapped{})
// Attempt to obtain an exclusive lock.
err := lockFileEx(syscall.Handle(fd), flag, 0, 1, 0, &syscall.Overlapped{})
if err == nil {
return nil
} else if err != errLockViolation {
return err
}

// If we timed oumercit then return an error.
if timeout != 0 && (maxSleep < 0 || time.Since(t) > maxSleep) {
return ErrTimeout
}

// Wait for a bit and try again.
time.Sleep(50 * time.Millisecond)
time.Sleep(timeoutStep)
}
}

Expand Down
3 changes: 3 additions & 0 deletions db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const (
// default page size for db is set to the OS page size.
var defaultPageSize = os.Getpagesize()

// The step used by the timeout mechanisms.
const timeoutStep = 50 * time.Millisecond
Copy link
Contributor

Choose a reason for hiding this comment

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

const flockRetryTimeout = 50 * time.Millisecond?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good for me. Will push an update


// DB represents a collection of buckets persisted to a file on disk.
// All data access is performed through transactions which can be obtained through the DB.
// All the functions on DB will return a ErrDatabaseNotOpen if accessed before Open() is called.
Expand Down