Slack vitess release patches

docker/base/Dockerfile

@@ -25,6 +25,9 @@ FROM vitess/bootstrap:mysql57
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.mariadb

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:mariadb
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.mariadb103

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:mariadb103
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.mysql56

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:mysql56
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.mysql80

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:mysql80
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.percona

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:percona
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.percona57

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:percona57
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/base/Dockerfile.percona80

@@ -1,5 +1,11 @@
 FROM vitess/bootstrap:percona80
 
+# Allows some docker builds to disable CGO
+ARG CGO_ENABLED=0
+
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree
 USER root
 COPY . /vt/src/vitess.io/vitess

docker/lite/Dockerfile.alpine

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:mariadb103 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.mariadb

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:mariadb AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.mariadb103

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:mariadb103 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.mysql56

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:mysql56 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.mysql57

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:mysql57 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.mysql80

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:mysql80 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.percona

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:percona AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.percona57

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:percona57 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

docker/lite/Dockerfile.percona80

@@ -22,6 +22,9 @@ FROM vitess/bootstrap:percona80 AS builder
 # Allows some docker builds to disable CGO
 ARG CGO_ENABLED=0
 
+# Allows docker builds to set the BUILD_NUMBER
+ARG BUILD_NUMBER
+
 # Re-copy sources from working tree.
 COPY --chown=vitess:vitess . /vt/src/vitess.io/vitess
 

go/sqltypes/result.go

@@ -159,13 +159,14 @@ func ResultsEqual(r1, r2 []Result) bool {
 // Every place this function is called, a comment is needed that explains
 // why it's justified.
 func MakeRowTrusted(fields []*querypb.Field, row *querypb.Row) []Value {
-	sqlRow := make([]Value, len(row.Lengths))
+	sqlRow := make([]Value, len(fields))
 	var offset int64
-	for i, length := range row.Lengths {
+	for i, fld := range fields {
+		length := row.Lengths[i]
 		if length < 0 {
 			continue
 		}
-		sqlRow[i] = MakeTrusted(fields[i].Type, row.Values[offset:offset+length])
+		sqlRow[i] = MakeTrusted(fld.Type, row.Values[offset:offset+length])
 		offset += length
 	}
 	return sqlRow

go/sqltypes/result_test.go

@@ -23,6 +23,61 @@ import (
 	querypb "vitess.io/vitess/go/vt/proto/query"
 )
 
+func TestMakeRowTrusted(t *testing.T) {
+	fields := MakeTestFields(
+		"some_int|some_text|another_int",
+		"int8|varchar|int8",
+	)
+
+	values := []byte{}
+	hw := []byte("hello, world")
+	values = append(values, hw...)
+	values = append(values, byte(42))
+
+	row := &querypb.Row{
+		Lengths: []int64{-1, int64(len(hw)), 1},
+		Values:  values,
+	}
+
+	want := []Value{
+		MakeTrusted(querypb.Type_NULL_TYPE, nil),
+		MakeTrusted(querypb.Type_VARCHAR, []byte("hello, world")),
+		MakeTrusted(querypb.Type_INT8, []byte{byte(42)}),
+	}
+
+	result := MakeRowTrusted(fields, row)
+	if !reflect.DeepEqual(result, want) {
+		t.Errorf("MakeRowTrusted:\ngot: %#v\nwant: %#v", result, want)
+	}
+}
+
+func TestMakeRowTrustedDoesNotPanicOnNewColumns(t *testing.T) {
+	fields := MakeTestFields(
+		"some_int|some_text",
+		"int8|varchar",
+	)
+
+	values := []byte{byte(123)}
+	hw := []byte("hello, world")
+	values = append(values, hw...)
+	values = append(values, byte(42))
+
+	row := &querypb.Row{
+		Lengths: []int64{1, int64(len(hw)), 1},
+		Values:  values,
+	}
+
+	want := []Value{
+		MakeTrusted(querypb.Type_INT8, []byte{byte(123)}),
+		MakeTrusted(querypb.Type_VARCHAR, []byte("hello, world")),
+	}
+
+	result := MakeRowTrusted(fields, row)
+	if !reflect.DeepEqual(result, want) {
+		t.Errorf("MakeRowTrusted:\ngot: %#v\nwant: %#v", result, want)
+	}
+}
+
 func TestRepair(t *testing.T) {
 	fields := []*querypb.Field{{
 		Type: Int64,

go/vt/mysqlctl/azblobbackupstorage/azblob.go

@@ -127,6 +127,21 @@ func (bh *AZBlobBackupHandle) Name() string {
 	return bh.name
 }
 
+// RecordError is part of the concurrency.ErrorRecorder interface.
+func (bh *AZBlobBackupHandle) RecordError(err error) {
+	bh.errors.RecordError(err)
+}
+
+// HasErrors is part of the concurrency.ErrorRecorder interface.
+func (bh *AZBlobBackupHandle) HasErrors() bool {
+	return bh.errors.HasErrors()
+}
+
+// Error is part of the concurrency.ErrorRecorder interface.
+func (bh *AZBlobBackupHandle) Error() error {
+	return bh.errors.Error()
+}
+
 // AddFile implements BackupHandle.
 func (bh *AZBlobBackupHandle) AddFile(ctx context.Context, filename string, filesize int64) (io.WriteCloser, error) {
 	if bh.readOnly {
@@ -156,7 +171,7 @@ func (bh *AZBlobBackupHandle) AddFile(ctx context.Context, filename string, file
 		})
 		if err != nil {
 			reader.CloseWithError(err)
-			bh.errors.RecordError(err)
+			bh.RecordError(err)
 		}
 	}()
 
@@ -169,7 +184,7 @@ func (bh *AZBlobBackupHandle) EndBackup(ctx context.Context) error {
 		return fmt.Errorf("EndBackup cannot be called on read-only backup")
 	}
 	bh.waitGroup.Wait()
-	return bh.errors.Error()
+	return bh.Error()
 }
 
 // AbortBackup implements BackupHandle.

go/vt/mysqlctl/backupstorage/interface.go

@@ -24,6 +24,7 @@ import (
 	"io"
 
 	"golang.org/x/net/context"
+	"vitess.io/vitess/go/vt/concurrency"
 )
 
 var (
@@ -74,6 +75,10 @@ type BackupHandle interface {
 	// The context is valid for the duration of the reads, until the
 	// ReadCloser is closed.
 	ReadFile(ctx context.Context, filename string) (io.ReadCloser, error)
+
+	// concurrency.ErrorRecorder is embedded here to coordinate reporting and
+	// handling of errors among all the components involved in taking a backup.
+	concurrency.ErrorRecorder
 }
 
 // BackupStorage is the interface to the storage system

go/vt/mysqlctl/builtinbackupengine.go

@@ -276,7 +276,6 @@ func (be *BuiltinBackupEngine) backupFiles(ctx context.Context, params BackupPar
 
 	// Backup with the provided concurrency.
 	sema := sync2.NewSemaphore(params.Concurrency, 0)
-	rec := concurrency.AllErrorRecorder{}
 	wg := sync.WaitGroup{}
 	for i := range fes {
 		wg.Add(1)
@@ -287,19 +286,28 @@ func (be *BuiltinBackupEngine) backupFiles(ctx context.Context, params BackupPar
 			// encountered an error.
 			sema.Acquire()
 			defer sema.Release()
-			if rec.HasErrors() {
+			if bh.HasErrors() {
 				return
 			}
 
 			// Backup the individual file.
 			name := fmt.Sprintf("%v", i)
-			rec.RecordError(be.backupFile(ctx, params, bh, &fes[i], name))
+			bh.RecordError(be.backupFile(ctx, params, bh, &fes[i], name))
 		}(i)
 	}
 
 	wg.Wait()
-	if rec.HasErrors() {
-		return rec.Error()
+
+	// BackupHandle supports the ErrorRecorder interface for tracking errors
+	// across any goroutines that fan out to take the backup. This means that we
+	// don't need a local error recorder and can put everything through the bh.
+	//
+	// This handles the scenario where bh.AddFile() encounters an error asynchronously,
+	// which ordinarily would be lost in the context of `be.backupFile`, i.e. if an
+	// error were encountered
+	// [here](https://github.com/vitessio/vitess/blob/d26b6c7975b12a87364e471e2e2dfa4e253c2a5b/go/vt/mysqlctl/s3backupstorage/s3.go#L139-L142).
+	if bh.HasErrors() {
+		return bh.Error()
 	}
 
 	// open the MANIFEST