diff --git a/src/cmd/go/internal/renameio/renameio_test.go b/src/cmd/go/internal/renameio/renameio_test.go index ace6e493cb..ee2f3ba1bb 100644 --- a/src/cmd/go/internal/renameio/renameio_test.go +++ b/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 { diff --git a/src/cmd/go/internal/robustio/robustio_darwin.go b/src/cmd/go/internal/robustio/robustio_darwin.go new file mode 100644 index 0000000000..99fd8ebc2f --- /dev/null +++ b/src/cmd/go/internal/robustio/robustio_darwin.go @@ -0,0 +1,21 @@ +// 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 +} diff --git a/src/cmd/go/internal/robustio/robustio_flaky.go b/src/cmd/go/internal/robustio/robustio_flaky.go new file mode 100644 index 0000000000..e57c8c74c4 --- /dev/null +++ b/src/cmd/go/internal/robustio/robustio_flaky.go @@ -0,0 +1,92 @@ +// 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) + }) +} diff --git a/src/cmd/go/internal/robustio/robustio_other.go b/src/cmd/go/internal/robustio/robustio_other.go index 56e6ad6d9c..907b556858 100644 --- a/src/cmd/go/internal/robustio/robustio_other.go +++ b/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 diff --git a/src/cmd/go/internal/robustio/robustio_windows.go b/src/cmd/go/internal/robustio/robustio_windows.go index a3d94e566f..687dcb66f8 100644 --- a/src/cmd/go/internal/robustio/robustio_windows.go +++ b/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 {