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

Open all vlog files in RDWR mode #923

Merged
merged 7 commits into from
Jul 16, 2019
Merged

Open all vlog files in RDWR mode #923

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
beed94d
Open all vlog files in RDWR mode
Jul 8, 2019
bebe11b
Fix typo
Jul 8, 2019
05a230b
Merge branch 'master' into ibrahim/vlog-readonly
Jul 10, 2019
b43aa16
Remove lock from donewriting function
Jul 10, 2019
94509b9
Add new line before function definition
Jul 10, 2019
b2f5cc3
Change vlog.openLogFile(..) => lf.Open(..)
Jul 11, 2019
798e913
Set sync flag for all files
Jul 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 41 additions & 74 deletions value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,29 +74,6 @@ type logFile struct {
loadingMode options.FileLoadingMode
}

// openReadOnly assumes that we have a write lock on logFile.
func (lf *logFile) openReadOnly() error {
var err error
lf.fd, err = os.OpenFile(lf.path, os.O_RDONLY, 0666)
if err != nil {
return errors.Wrapf(err, "Unable to open %q as RDONLY.", lf.path)
}

fi, err := lf.fd.Stat()
if err != nil {
return errors.Wrapf(err, "Unable to check stat for %q", lf.path)
}
y.AssertTrue(fi.Size() <= math.MaxUint32)
lf.size = uint32(fi.Size())

if err = lf.mmap(fi.Size()); err != nil {
_ = lf.fd.Close()
return y.Wrapf(err, "Unable to map file: %q", fi.Name())
}

return nil
}

func (lf *logFile) mmap(size int64) (err error) {
if lf.loadingMode != options.MemoryMap {
// Nothing to do
Expand Down Expand Up @@ -148,33 +125,21 @@ func (lf *logFile) read(p valuePointer, s *y.Slice) (buf []byte, err error) {
}

func (lf *logFile) doneWriting(offset uint32) error {
// Sync before acquiring lock. (We call this from write() and thus know we have shared access
// Sync before acquiring lock. (We call this from write() and thus know we have shared access
// to the fd.)
if err := y.FileSync(lf.fd); err != nil {
return errors.Wrapf(err, "Unable to sync value log: %q", lf.path)
}
// Close and reopen the file read-only. Acquire lock because fd will become invalid for a bit.
// Acquiring the lock is bad because, while we don't hold the lock for a long time, it forces
// one batch of readers wait for the preceding batch of readers to finish.
//
// If there's a benefit to reopening the file read-only, it might be on Windows. I don't know
// what the benefit is. Consider keeping the file read-write, or use fcntl to change
// permissions.
lf.lock.Lock()
defer lf.lock.Unlock()
if err := lf.munmap(); err != nil {
return err
}

// TODO: Confirm if we need to run a file sync after truncation.
// Truncation must run after unmapping, otherwise Windows would crap itself.
if err := lf.fd.Truncate(int64(offset)); err != nil {
return errors.Wrapf(err, "Unable to truncate file: %q", lf.path)
}
if err := lf.fd.Close(); err != nil {
return errors.Wrapf(err, "Unable to close value log: %q", lf.path)
}

return lf.openReadOnly()
// Previously we used to close the file after it was written and reopen it in read-only mode.
// We no longer open files in read-only mode. We keep all vlog files open in read-write mode.
return nil
}

// You must hold lf.lock to sync()
Expand Down Expand Up @@ -717,18 +682,6 @@ func errFile(err error, path string, msg string) error {
}

func (vlog *valueLog) replayLog(lf *logFile, offset uint32, replayFn logEntry) error {
var err error
mode := os.O_RDONLY
if vlog.opt.Truncate {
// We should open the file in RW mode, so it can be truncated.
mode = os.O_RDWR
}
lf.fd, err = os.OpenFile(lf.path, mode, 0)
if err != nil {
return errFile(err, lf.path, "Open file")
}
defer lf.fd.Close()

fi, err := lf.fd.Stat()
if err != nil {
return errFile(err, lf.path, "Unable to run file.Stat")
Expand Down Expand Up @@ -785,13 +738,23 @@ func (vlog *valueLog) open(db *DB, ptr valuePointer, replayFn logEntry) error {
for _, fid := range fids {
lf, ok := vlog.filesMap[fid]
y.AssertTrue(ok)
var flags uint32
switch {
case vlog.opt.ReadOnly:
// If we have read only, we don't need SyncWrites.
flags |= y.ReadOnly
// Set sync flag only for the last vlog file.
case vlog.opt.SyncWrites && lf.fid == vlog.maxFid:
flags |= y.Sync
}

// Open log file "lf" in read-write mode.
if err := lf.open(vlog.fpath(lf.fid), flags); err != nil {
return err
}
// This file is before the value head pointer. So, we don't need to
// replay it, and can just open it in readonly mode.
if fid < ptr.Fid {
if err := lf.openReadOnly(); err != nil {
return err
}
continue
}

Expand All @@ -816,25 +779,6 @@ func (vlog *valueLog) open(db *DB, ptr valuePointer, replayFn logEntry) error {
return err
}
vlog.db.opt.Infof("Replay took: %s\n", time.Since(now))

if fid < vlog.maxFid {
if err := lf.openReadOnly(); err != nil {
return err
}
} else {
var flags uint32
switch {
case vlog.opt.ReadOnly:
// If we have read only, we don't need SyncWrites.
flags |= y.ReadOnly
case vlog.opt.SyncWrites:
flags |= y.Sync
}
var err error
if lf.fd, err = y.OpenExistingFile(vlog.fpath(fid), flags); err != nil {
return errFile(err, lf.path, "Open existing file")
}
}
}

// Seek to the end to start writing.
Expand All @@ -861,6 +805,29 @@ func (vlog *valueLog) open(db *DB, ptr valuePointer, replayFn logEntry) error {
return nil
}

func (lf *logFile) open(filename string, flags uint32) error {
var err error
if lf.fd, err = y.OpenExistingFile(filename, flags); err != nil {
return errors.Wrapf(err, "Open existing file: %q", lf.path)
}
fstat, err := lf.fd.Stat()
if err != nil {
return errors.Wrapf(err, "Unable to check stat for %q", lf.path)
}
sz := fstat.Size()
if sz == 0 {
// File is empty. We don't need to mmap it. Return.
return nil
}
y.AssertTrue(sz <= math.MaxUint32)
lf.size = uint32(sz)
if err = lf.mmap(sz); err != nil {
_ = lf.fd.Close()
return errors.Wrapf(err, "Unable to map file: %q", fstat.Name())
}
return nil
}

func (vlog *valueLog) Close() error {
vlog.elog.Printf("Stopping garbage collection of values.")
defer vlog.elog.Finish()
Expand Down