Support no timeout locks on db files #35
Conversation
Codecov Report
@@ Coverage Diff @@
## master #35 +/- ##
==========================================
+ Coverage 85.01% 85.56% +0.55%
==========================================
Files 9 9
Lines 1848 1857 +9
==========================================
+ Hits 1571 1589 +18
+ Misses 164 159 -5
+ Partials 113 109 -4
Continue to review full report at Codecov.
|
db.go
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
const flockRetryTimeout = 50 * time.Millisecond?
There was a problem hiding this comment.
Good for me. Will push an update
bolt_unix_solaris.go
Outdated
| 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) { |
There was a problem hiding this comment.
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)
}There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think relying on >=1.9 is OK since this is a new feature that won't interfere with old behavior.
|
I implemented the proposed changes |
|
Will you use github's Squash and merge feature to squash the commits? |
gyuho
changed the title
This is continuation of pull request #34 with same name, itself derived from boltdb/bolt#494
I implemented the changes proposed by @heyitsanthony. I ended up with conflicts between my version derived from boltdb/bolt and the coreos/bbolt master. I resolved them and committed them into a clean branch based on coreos/bbolt master