Merge #41901 #41993

41901: storage/engine: centralize specification of pebble.Options r=petermattis a=petermattis

Fixes #41860

Release note: None

41993: build: Upgrade to go 1.12.12 r=bobvawter a=bobvawter

This change upgrades the go runtime to 1.12.12 in order to pick up a [security
fix](golang/go#34960).

Per the [checklist](build/README.md):
* [X] Adjust version in Docker image
* [X] Rebuild the Docker image and bump the version in builder.sh accordingly
* [ ] ~Bump the version in go-version-check.sh~ (Patch release, not necessary)
* [X] Bump the default installed version of Go in bootstrap-debian.sh

Fixes: #41718

Release note (build change): The go runtime has been upgraded to 1.12.12.

Co-authored-by: Peter Mattis <[email protected]>
Co-authored-by: Bob Vawter <[email protected]>

Author: 3 people

build/bootstrap/bootstrap-debian.sh

@@ -38,9 +38,9 @@ echo '. ~/.bashrc_bootstrap' >> ~/.bashrc
 
 # Install Go.
 trap 'rm -f /tmp/go.tgz' EXIT
-curl https://dl.google.com/go/go1.12.10.linux-amd64.tar.gz > /tmp/go.tgz
+curl https://dl.google.com/go/go1.12.12.linux-amd64.tar.gz > /tmp/go.tgz
 sha256sum -c - <<EOF
-aaa84147433aed24e70b31da369bb6ca2859464a45de47c2a5023d8573412f6b  /tmp/go.tgz
+4cf11ac6a8fa42d26ab85e27a5d916ee171900a87745d9f7d4a29a21587d78fc  /tmp/go.tgz
 EOF
 sudo tar -C /usr/local -zxf /tmp/go.tgz
 

build/builder.sh

@@ -3,7 +3,7 @@
 set -euo pipefail
 
 image=cockroachdb/builder
-version=20191014-135449
+version=20191029-105705
 
 function init() {
   docker build --tag="${image}" "$(dirname "${0}")/builder"

build/builder/Dockerfile

@@ -196,8 +196,8 @@ RUN git clone git://git.sv.gnu.org/sed \
 # releases of Go will no longer be run in CI once it is changed. Consider
 # bumping the minimum allowed version of Go in /build/go-version-chech.sh.
 RUN apt-get install -y --no-install-recommends golang \
- && curl -fsSL https://storage.googleapis.com/golang/go1.12.10.src.tar.gz -o golang.tar.gz \
- && echo 'f56e48fce80646d3c94dcf36d3e3f490f6d541a92070ad409b87b6bbb9da3954 golang.tar.gz' | sha256sum -c - \
+ && curl -fsSL https://storage.googleapis.com/golang/go1.12.12.src.tar.gz -o golang.tar.gz \
+ && echo 'fcb33b5290fa9bcc52be3211501540df7483d7276b031fc77528672a3c705b99 golang.tar.gz' | sha256sum -c - \
  && tar -C /usr/local -xzf golang.tar.gz \
  && rm golang.tar.gz \
  && cd /usr/local/go/src \

pkg/base/config.go

@@ -659,6 +659,9 @@ type StorageConfig struct {
 	// Makes no sense for in-memory instances.
 	// TODO(hueypark): Implement this for pebble.
 	MustExist bool
+	// MaxSize is used for calculating free space and making rebalancing
+	// decisions. Zero indicates that there is no maximum size.
+	MaxSize int64
 	// Settings instance for cluster-wide knobs.
 	Settings *cluster.Settings
 	// UseFileRegistry is true if the file registry is needed (eg: encryption-at-rest).

pkg/server/config.go

@@ -498,6 +498,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) {
 			storageConfig := base.StorageConfig{
 				Attrs:           spec.Attributes,
 				Dir:             spec.Path,
+				MaxSize:         sizeInBytes,
 				Settings:        cfg.Settings,
 				UseFileRegistry: spec.UseFileRegistry,
 				ExtraOptions:    spec.ExtraOptions,
@@ -507,25 +508,14 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) {
 				// in the spec (similar to the existing spec.RocksDBOptions and others).
 				pebbleConfig := engine.PebbleConfig{
 					StorageConfig: storageConfig,
-					Opts: &pebble.Options{
-						Cache:                       pebbleCache,
-						MaxOpenFiles:                int(openFileLimitPerStore),
-						MemTableSize:                64 << 20,
-						MemTableStopWritesThreshold: 4,
-						MinFlushRate:                4 << 20,
-						L0CompactionThreshold:       2,
-						L0StopWritesThreshold:       400,
-						LBaseMaxBytes:               64 << 20, // 64 MB
-						Levels: []pebble.LevelOptions{{
-							BlockSize: 32 << 10,
-						}},
-					},
+					Opts:          engine.DefaultPebbleOptions(),
 				}
+				pebbleConfig.Opts.Cache = pebbleCache
+				pebbleConfig.Opts.MaxOpenFiles = int(openFileLimitPerStore)
 				eng, err = engine.NewPebble(pebbleConfig)
 			} else {
 				rocksDBConfig := engine.RocksDBConfig{
 					StorageConfig:           storageConfig,
-					MaxSizeBytes:            sizeInBytes,
 					MaxOpenFiles:            openFileLimitPerStore,
 					WarnLargeBatchThreshold: 500 * time.Millisecond,
 					RocksDBOptions:          spec.RocksDBOptions,

pkg/storage/engine/bench_pebble_test.go

@@ -20,28 +20,19 @@ import (
 	"github.com/cockroachdb/pebble/vfs"
 )
 
-func newPebbleOptions(fs vfs.FS) *pebble.Options {
-	return &pebble.Options{
-		Cache:                       pebble.NewCache(testCacheSize),
-		FS:                          fs,
-		MemTableSize:                64 << 20,
-		MemTableStopWritesThreshold: 4,
-		MinFlushRate:                4 << 20,
-		L0CompactionThreshold:       2,
-		L0StopWritesThreshold:       400,
-		LBaseMaxBytes:               64 << 20, // 64 MB
-		Levels: []pebble.LevelOptions{{
-			BlockSize: 32 << 10,
-		}},
-	}
+func testPebbleOptions(fs vfs.FS) *pebble.Options {
+	opts := DefaultPebbleOptions()
+	opts.Cache = pebble.NewCache(testCacheSize)
+	opts.FS = fs
+	return opts
 }
 
 func setupMVCCPebble(b testing.TB, dir string) Engine {
 	peb, err := NewPebble(PebbleConfig{
 		StorageConfig: base.StorageConfig{
 			Dir: dir,
 		},
-		Opts: newPebbleOptions(vfs.Default),
+		Opts: testPebbleOptions(vfs.Default),
 	})
 	if err != nil {
 		b.Fatalf("could not create new pebble instance at %s: %+v", dir, err)
@@ -51,7 +42,7 @@ func setupMVCCPebble(b testing.TB, dir string) Engine {
 
 func setupMVCCInMemPebble(b testing.TB, loc string) Engine {
 	peb, err := NewPebble(PebbleConfig{
-		Opts: newPebbleOptions(vfs.NewMem()),
+		Opts: testPebbleOptions(vfs.NewMem()),
 	})
 	if err != nil {
 		b.Fatalf("could not create new in-mem pebble instance: %+v", err)

pkg/storage/engine/engine_test.go

@@ -31,7 +31,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
-	"github.com/cockroachdb/pebble"
 	"github.com/cockroachdb/pebble/vfs"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -70,9 +69,7 @@ func runWithAllEngines(test func(e Engine, t *testing.T), t *testing.T) {
 
 	func() {
 		pebbleInMem, err := NewPebble(PebbleConfig{
-			Opts: &pebble.Options{
-				FS: vfs.NewMem(),
-			},
+			Opts: testPebbleOptions(vfs.NewMem()),
 		})
 		if err != nil {
 			t.Fatal(err)

pkg/storage/engine/mvcc_test.go

@@ -37,7 +37,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/shuffle"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
-	"github.com/cockroachdb/pebble"
 	"github.com/cockroachdb/pebble/vfs"
 	"github.com/gogo/protobuf/proto"
 	"github.com/kr/pretty"
@@ -89,9 +88,7 @@ func createTestRocksDBEngine() Engine {
 // createTestPebbleEngine returns a new in-memory Pebble storage engine.
 func createTestPebbleEngine() Engine {
 	peb, err := NewPebble(PebbleConfig{
-		Opts: &pebble.Options{
-			FS: vfs.NewMem(),
-		},
+		Opts: testPebbleOptions(vfs.NewMem()),
 	})
 	if err != nil {
 		return nil

pkg/storage/engine/pebble.go

@@ -186,11 +186,29 @@ var PebbleTablePropertyCollectors = []func() pebble.TablePropertyCollector{
 	func() pebble.TablePropertyCollector { return &pebbleDeleteRangeCollector{} },
 }
 
+// DefaultPebbleOptions returns the default pebble options.
+func DefaultPebbleOptions() *pebble.Options {
+	return &pebble.Options{
+		Comparer:              MVCCComparer,
+		L0CompactionThreshold: 2,
+		L0StopWritesThreshold: 1000,
+		LBaseMaxBytes:         64 << 20, // 64 MB
+		Levels: []pebble.LevelOptions{{
+			BlockSize: 32 << 10, // 32 KB
+		}},
+		MemTableSize:                64 << 20, // 64 MB
+		MemTableStopWritesThreshold: 4,
+		Merger:                      MVCCMerger,
+		MinFlushRate:                4 << 20, // 4 MB/sec
+		TablePropertyCollectors:     PebbleTablePropertyCollectors,
+	}
+}
+
 // PebbleConfig holds all configuration parameters and knobs used in setting up
 // a new Pebble instance.
 type PebbleConfig struct {
 	// StorageConfig contains storage configs for all storage engines.
-	StorageConfig base.StorageConfig
+	base.StorageConfig
 	// Pebble specific options.
 	Opts *pebble.Options
 }
@@ -201,6 +219,7 @@ type Pebble struct {
 
 	closed   bool
 	path     string
+	maxSize  int64
 	attrs    roachpb.Attributes
 	settings *cluster.Settings
 
@@ -212,10 +231,6 @@ var _ Engine = &Pebble{}
 
 // NewPebble creates a new Pebble instance, at the specified path.
 func NewPebble(cfg PebbleConfig) (*Pebble, error) {
-	cfg.Opts.Comparer = MVCCComparer
-	cfg.Opts.Merger = MVCCMerger
-	cfg.Opts.TablePropertyCollectors = PebbleTablePropertyCollectors
-
 	// pebble.Open also calls EnsureDefaults, but only after doing a clone. Call
 	// EnsureDefaults beforehand so we have a matching cfg here for when we save
 	// cfg.FS and cfg.ReadOnly later on.
@@ -228,9 +243,10 @@ func NewPebble(cfg PebbleConfig) (*Pebble, error) {
 
 	return &Pebble{
 		db:       db,
-		path:     cfg.StorageConfig.Dir,
-		attrs:    cfg.StorageConfig.Attrs,
-		settings: cfg.StorageConfig.Settings,
+		path:     cfg.Dir,
+		maxSize:  cfg.MaxSize,
+		attrs:    cfg.Attrs,
+		settings: cfg.Settings,
 		fs:       cfg.Opts.FS,
 	}, nil
 }
@@ -389,9 +405,7 @@ func (p *Pebble) Attrs() roachpb.Attributes {
 
 // Capacity implements the Engine interface.
 func (p *Pebble) Capacity() (roachpb.StoreCapacity, error) {
-	// Pebble doesn't have a capacity limiting parameter, so pass 0 for
-	// maxSizeBytes to denote no limit.
-	return computeCapacity(p.path, 0)
+	return computeCapacity(p.path, p.maxSize)
 }
 
 // Flush implements the Engine interface.