cmd/go/internal/robustio: extend filesystem workarounds to darwin platforms

The macOS filesystem seems to have gotten significantly flakier as of
macOS 10.14, so this causes frequently flakes in the 10.14 builders.

We have no reason to believe that it will be fixed any time soon, so
rather than trying to detect the specific macOS version, we'll apply
the same workarounds that we use on Windows: classifying (and
retrying) the errors known to indicate flakiness and relaxing the
success criteria for renameio.TestConcurrentReadsAndWrites.

Fixes #33041

Change-Id: I74d8c15677951d7a0df0d4ebf6ea03e43eebddf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197517
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

src/cmd/go/internal/renameio/renameio_test.go

@@ -131,10 +131,18 @@ func TestConcurrentReadsAndWrites(t *testing.T) {
 	}
 
 	var minReadSuccesses int64 = attempts
-	if runtime.GOOS == "windows" {
+
+	switch runtime.GOOS {
+	case "windows":
 		// Windows produces frequent "Access is denied" errors under heavy rename load.
-		// As long as those are the only errors and *some* of the writes succeed, we're happy.
+		// As long as those are the only errors and *some* of the reads succeed, we're happy.
 		minReadSuccesses = attempts / 4
+
+	case "darwin":
+		// The filesystem on macOS 10.14 occasionally fails with "no such file or
+		// directory" errors. See https://golang.org/issue/33041. The flake rate is
+		// fairly low, so ensure that at least 75% of attempts succeed.
+		minReadSuccesses = attempts - (attempts / 4)
 	}
 
 	if readSuccesses < minReadSuccesses {

src/cmd/go/internal/robustio/robustio_darwin.go

+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package robustio
+
+import (
+	"errors"
+	"syscall"
+)
+
+const errFileNotFound = syscall.ENOENT
+
+// isEphemeralError returns true if err may be resolved by waiting.
+func isEphemeralError(err error) bool {
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		return errno == errFileNotFound
+	}
+	return false
+}

src/cmd/go/internal/robustio/robustio_flaky.go

+// Copyright 2019 The Go 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 windows darwin
+
+package robustio
+
+import (
+	"errors"
+	"io/ioutil"
+	"math/rand"
+	"os"
+	"syscall"
+	"time"
+)
+
+const arbitraryTimeout = 500 * time.Millisecond
+
+// retry retries ephemeral errors from f up to an arbitrary timeout
+// to work around filesystem flakiness on Windows and Darwin.
+func retry(f func() (err error, mayRetry bool)) error {
+	var (
+		bestErr     error
+		lowestErrno syscall.Errno
+		start       time.Time
+		nextSleep   time.Duration = 1 * time.Millisecond
+	)
+	for {
+		err, mayRetry := f()
+		if err == nil || !mayRetry {
+			return err
+		}
+
+		var errno syscall.Errno
+		if errors.As(err, &errno) && (lowestErrno == 0 || errno < lowestErrno) {
+			bestErr = err
+			lowestErrno = errno
+		} else if bestErr == nil {
+			bestErr = err
+		}
+
+		if start.IsZero() {
+			start = time.Now()
+		} else if d := time.Since(start) + nextSleep; d >= arbitraryTimeout {
+			break
+		}
+		time.Sleep(nextSleep)
+		nextSleep += time.Duration(rand.Int63n(int64(nextSleep)))
+	}
+
+	return bestErr
+}
+
+// rename is like os.Rename, but retries ephemeral errors.
+//
+// On Windows it wraps os.Rename, which (as of 2019-06-04) uses MoveFileEx with
+// MOVEFILE_REPLACE_EXISTING.
+//
+// Windows also provides a different system call, ReplaceFile,
+// that provides similar semantics, but perhaps preserves more metadata. (The
+// documentation on the differences between the two is very sparse.)
+//
+// Empirical error rates with MoveFileEx are lower under modest concurrency, so
+// for now we're sticking with what the os package already provides.
+func rename(oldpath, newpath string) (err error) {
+	return retry(func() (err error, mayRetry bool) {
+		err = os.Rename(oldpath, newpath)
+		return err, isEphemeralError(err)
+	})
+}
+
+// readFile is like ioutil.ReadFile, but retries ephemeral errors.
+func readFile(filename string) ([]byte, error) {
+	var b []byte
+	err := retry(func() (err error, mayRetry bool) {
+		b, err = ioutil.ReadFile(filename)
+
+		// Unlike in rename, we do not retry errFileNotFound here: it can occur
+		// as a spurious error, but the file may also genuinely not exist, so the
+		// increase in robustness is probably not worth the extra latency.
+		return err, isEphemeralError(err) && !errors.Is(err, errFileNotFound)
+	})
+	return b, err
+}
+
+func removeAll(path string) error {
+	return retry(func() (err error, mayRetry bool) {
+		err = os.RemoveAll(path)
+		return err, isEphemeralError(err)
+	})
+}

src/cmd/go/internal/robustio/robustio_other.go

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build !windows
+// +build !windows,!darwin
 
 package robustio
 

src/cmd/go/internal/robustio/robustio_windows.go

@@ -7,88 +7,10 @@ package robustio
 import (
 	"errors"
 	"internal/syscall/windows"
-	"io/ioutil"
-	"math/rand"
-	"os"
 	"syscall"
-	"time"
 )
 
-const arbitraryTimeout = 500 * time.Millisecond
-
-// retry retries ephemeral errors from f up to an arbitrary timeout
-// to work around spurious filesystem errors on Windows
-func retry(f func() (err error, mayRetry bool)) error {
-	var (
-		bestErr     error
-		lowestErrno syscall.Errno
-		start       time.Time
-		nextSleep   time.Duration = 1 * time.Millisecond
-	)
-	for {
-		err, mayRetry := f()
-		if err == nil || !mayRetry {
-			return err
-		}
-
-		var errno syscall.Errno
-		if errors.As(err, &errno) && (lowestErrno == 0 || errno < lowestErrno) {
-			bestErr = err
-			lowestErrno = errno
-		} else if bestErr == nil {
-			bestErr = err
-		}
-
-		if start.IsZero() {
-			start = time.Now()
-		} else if d := time.Since(start) + nextSleep; d >= arbitraryTimeout {
-			break
-		}
-		time.Sleep(nextSleep)
-		nextSleep += time.Duration(rand.Int63n(int64(nextSleep)))
-	}
-
-	return bestErr
-}
-
-// rename is like os.Rename, but retries ephemeral errors.
-//
-// It wraps os.Rename, which (as of 2019-06-04) uses MoveFileEx with
-// MOVEFILE_REPLACE_EXISTING.
-//
-// Windows also provides a different system call, ReplaceFile,
-// that provides similar semantics, but perhaps preserves more metadata. (The
-// documentation on the differences between the two is very sparse.)
-//
-// Empirical error rates with MoveFileEx are lower under modest concurrency, so
-// for now we're sticking with what the os package already provides.
-func rename(oldpath, newpath string) (err error) {
-	return retry(func() (err error, mayRetry bool) {
-		err = os.Rename(oldpath, newpath)
-		return err, isEphemeralError(err)
-	})
-}
-
-// readFile is like ioutil.ReadFile, but retries ephemeral errors.
-func readFile(filename string) ([]byte, error) {
-	var b []byte
-	err := retry(func() (err error, mayRetry bool) {
-		b, err = ioutil.ReadFile(filename)
-
-		// Unlike in rename, we do not retry ERROR_FILE_NOT_FOUND here: it can occur
-		// as a spurious error, but the file may also genuinely not exist, so the
-		// increase in robustness is probably not worth the extra latency.
-		return err, isEphemeralError(err) && !errors.Is(err, syscall.ERROR_FILE_NOT_FOUND)
-	})
-	return b, err
-}
-
-func removeAll(path string) error {
-	return retry(func() (err error, mayRetry bool) {
-		err = os.RemoveAll(path)
-		return err, isEphemeralError(err)
-	})
-}
+const errFileNotFound = syscall.ERROR_FILE_NOT_FOUND
 
 // isEphemeralError returns true if err may be resolved by waiting.
 func isEphemeralError(err error) bool {