Repository: petermattis/pebble
Pull request #215 (2019-08-08 20:11:35)

vfs, table_cache: fadvise FADV_RANDOM on sstable files
This change calls fadvise with FADV_RANDOM on sstable file descriptors,
to ensure that readahead is disabled. This reduces wasted I/Os when
reading from sstables, since sstable reads especially for short to
medium range scans do not read large enough contiguous blocks to be
able to take advantage of readahead. Instead, readahead ends up reducing
user-visible I/O performance.

See #198 .

petermattis: 

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.png" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/petermattis/pebble/215)
<!-- Reviewable:end -->

itsbilal: Some comparative benchmarks, command run (on a c5d.4xlarge) with 26gb malloc'd away by another program (see issue #198 for repro steps):

```
./pebble ycsb /mnt/data1/bench_16G --workload scan=100 --concurrency 16 --prepopulated-keys $[16*1024*1024*1024/64] --initial-keys 0 --keys uniform --duration 2m --cache $[1*1024*1024*1024] --scans 100
```

Pebble (before change):

```
____optype__elapsed_____ops(total)___ops/sec(cum)__keys/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)
  scan_100   120.0s         303419         2527.6      2527558.9      6.3      6.3     10.0     21.0     65.0
```

Pebble (after change):

```
____optype__elapsed_____ops(total)___ops/sec(cum)__keys/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)
  scan_100   120.0s         791940         6599.5      6599460.5      2.4      2.4      3.4      5.8    192.9
```

RocksDB (run 1):

```
____optype__elapsed_____ops(total)___ops/sec(cum)__keys/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)
  scan_100   120.0s         785964         6549.6      6549625.3      2.4      2.5      3.3      4.2    184.5
```

RocksDB (run 2):

```
____optype__elapsed_____ops(total)___ops/sec(cum)__keys/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)
  scan_100   120.3s         766692         6374.0      6374020.5      2.5      2.5      3.4      6.6     88.1
```

Patch:
diff --git a/go.mod b/go.mod
index 0e59345..a1a1155 100644
--- a/go.mod
+++ b/go.mod
@@ -11,4 +11,5 @@ require (
 	github.com/spf13/pflag v1.0.3 // indirect
 	github.com/stretchr/testify v1.2.2
 	golang.org/x/exp v0.0.0-20190426190305-956cc1757749
+	golang.org/x/sys v0.0.0-20190804053845-51ab0e2deafa // indirect
 )
diff --git a/go.sum b/go.sum
index f91daa2..edcb053 100644
--- a/go.sum
+++ b/go.sum
@@ -28,5 +28,7 @@ golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20190804053845-51ab0e2deafa h1:KIDDMLT1O0Nr7TSxp8xM5tJcdn8tgyAONntO829og1M=
+golang.org/x/sys v0.0.0-20190804053845-51ab0e2deafa/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20190312151545-0bb0c0a6e846/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
diff --git a/table_cache.go b/table_cache.go
index 1f1a0ad..cefa10b 100644
--- a/table_cache.go
+++ b/table_cache.go
@@ -327,7 +327,7 @@ type tableCacheNode struct {
 
 func (n *tableCacheNode) load(c *tableCacheShard) {
 	// Try opening the fileTypeTable first.
-	f, err := c.fs.Open(dbFilename(c.dirname, fileTypeTable, n.meta.fileNum))
+	f, err := c.fs.Open(dbFilename(c.dirname, fileTypeTable, n.meta.fileNum), vfs.RandomReadsOption)
 	if err != nil {
 		n.err = err
 		close(n.loaded)
diff --git a/vendor/modules.txt b/vendor/modules.txt
index edfc563..9533257 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -21,3 +21,5 @@ github.com/stretchr/testify/require
 github.com/stretchr/testify/assert
 # golang.org/x/exp v0.0.0-20190426190305-956cc1757749
 golang.org/x/exp/rand
+# golang.org/x/sys v0.0.0-20190804053845-51ab0e2deafa
+golang.org/x/sys
diff --git a/vfs/fadvise_generic.go b/vfs/fadvise_generic.go
new file mode 100644
index 0000000..b7aa995
--- /dev/null
+++ b/vfs/fadvise_generic.go
@@ -0,0 +1,11 @@
+// Copyright 2019 The LevelDB-Go and Pebble Authors. All rights reserved. Use
+// of this source code is governed by a BSD-style license that can be found in
+// the LICENSE file.
+
+// +build !linux
+
+package vfs
+
+func fadviseRandom(f uintptr) error {
+	return nil
+}
diff --git a/vfs/fadvise_linux.go b/vfs/fadvise_linux.go
new file mode 100644
index 0000000..05d611d
--- /dev/null
+++ b/vfs/fadvise_linux.go
@@ -0,0 +1,14 @@
+// Copyright 2019 The LevelDB-Go and Pebble Authors. All rights reserved. Use
+// of this source code is governed by a BSD-style license that can be found in
+// the LICENSE file.
+
+// +build linux
+
+package vfs
+
+import "golang.org/x/sys/unix"
+
+// Calls Fadvise with FADV_RANDOM to disable readahead on a file descriptor.
+func fadviseRandom(f uintptr) error {
+	return unix.Fadvise(int(f), 0, 0, unix.FADV_RANDOM)
+}
diff --git a/vfs/mem_fs.go b/vfs/mem_fs.go
index 7ddc89b..088ac1a 100644
--- a/vfs/mem_fs.go
+++ b/vfs/mem_fs.go
@@ -40,6 +40,8 @@ type memFS struct {
 	root *memNode
 }
 
+var _ FS = &memFS{}
+
 func (y *memFS) String() string {
 	y.mu.Lock()
 	defer y.mu.Unlock()
@@ -190,7 +192,7 @@ func (y *memFS) open(fullname string, allowEmptyName bool) (File, error) {
 	return ret, nil
 }
 
-func (y *memFS) Open(fullname string) (File, error) {
+func (y *memFS) Open(fullname string, opts ...OpenOption) (File, error) {
 	return y.open(fullname, false /* allowEmptyName */)
 }
 
diff --git a/vfs/vfs.go b/vfs/vfs.go
index 831b811..a6ed601 100644
--- a/vfs/vfs.go
+++ b/vfs/vfs.go
@@ -23,6 +23,13 @@ type File interface {
 	Sync() error
 }
 
+// OpenOptions provide an interface to do work on file handles in the Open()
+// call.
+type OpenOption interface {
+	// Apply is called on the file handle after it's opened.
+	Apply(File)
+}
+
 // FS is a namespace for files.
 //
 // The names are filepath names: they may be / separated or \ separated,
@@ -35,8 +42,8 @@ type FS interface {
 	// Link creates newname as a hard link to the oldname file.
 	Link(oldname, newname string) error
 
-	// Open opens the named file for reading.
-	Open(name string) (File, error)
+	// Open opens the named file for reading. openOptions provides
+	Open(name string, opts ...OpenOption) (File, error)
 
 	// OpenDir opens the named directory for syncing.
 	OpenDir(name string) (File, error)
@@ -95,8 +102,15 @@ func (defaultFS) Link(oldname, newname string) error {
 	return os.Link(oldname, newname)
 }
 
-func (defaultFS) Open(name string) (File, error) {
-	return os.OpenFile(name, os.O_RDONLY|syscall.O_CLOEXEC, 0)
+func (defaultFS) Open(name string, opts ...OpenOption) (File, error) {
+	file, err := os.OpenFile(name, os.O_RDONLY|syscall.O_CLOEXEC, 0)
+	if err != nil {
+		return nil, err
+	}
+	for _, opt := range opts {
+		opt.Apply(file)
+	}
+	return file, nil
 }
 
 func (defaultFS) OpenDir(name string) (File, error) {
@@ -127,3 +141,18 @@ func (defaultFS) List(dir string) ([]string, error) {
 func (defaultFS) Stat(name string) (os.FileInfo, error) {
 	return os.Stat(name)
 }
+
+
+type randomReadsOption struct{}
+
+// RandomReadsOption is an OpenOption that optimizes opened file handle for
+// random reads, by calling  fadvise() with POSIX_FADV_RANDOM on Linux systems
+// to disable readahead. Only works when specified to defaultFS.
+var RandomReadsOption OpenOption = &randomReadsOption{}
+
+// Apply implements the OpenOption interface.
+func (randomReadsOption) Apply(f File) {
+	if osFile, ok := f.(*os.File); ok {
+		_ = fadviseRandom(osFile.Fd())
+	}
+}


Test patch:
diff --git a/error_test.go b/error_test.go
index 710de8b..59f600b 100644
--- a/error_test.go
+++ b/error_test.go
@@ -60,7 +60,7 @@ func (fs *errorFS) Link(oldname, newname string) error {
 	return fs.fs.Link(oldname, newname)
 }
 
-func (fs *errorFS) Open(name string) (vfs.File, error) {
+func (fs *errorFS) Open(name string, opts ...vfs.OpenOption) (vfs.File, error) {
 	if err := fs.maybeError(); err != nil {
 		return nil, err
 	}
@@ -68,7 +68,11 @@ func (fs *errorFS) Open(name string) (vfs.File, error) {
 	if err != nil {
 		return nil, err
 	}
-	return errorFile{f, fs}, nil
+	ef := errorFile{f, fs}
+	for _, opt := range opts {
+		opt.Apply(ef)
+	}
+	return ef, nil
 }
 
 func (fs *errorFS) OpenDir(name string) (vfs.File, error) {
diff --git a/table_cache_test.go b/table_cache_test.go
index 1a22c23..46d86e2 100644
--- a/table_cache_test.go
+++ b/table_cache_test.go
@@ -41,13 +41,16 @@ type tableCacheTestFS struct {
 	closeCounts map[string]int
 }
 
-func (fs *tableCacheTestFS) Open(name string) (vfs.File, error) {
+func (fs *tableCacheTestFS) Open(name string, opts ...vfs.OpenOption) (vfs.File, error) {
 	fs.mu.Lock()
 	if fs.openCounts != nil {
 		fs.openCounts[name]++
 	}
 	fs.mu.Unlock()
-	f, err := fs.FS.Open(name)
+	f, err := fs.FS.Open(name, opts...)
+	if len(opts) < 1 || opts[0] != vfs.RandomReadsOption {
+		return nil, fmt.Errorf("sstable file %s not opened with random reads option", name)
+	}
 	if err != nil {
 		return nil, err
 	}