]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/go/internal/robustio: extend filesystem workarounds to darwin platforms
authorBryan C. Mills <bcmills@google.com>
Thu, 26 Sep 2019 14:22:26 +0000 (10:22 -0400)
committerBryan C. Mills <bcmills@google.com>
Thu, 26 Sep 2019 15:28:04 +0000 (15:28 +0000)
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
src/cmd/go/internal/robustio/robustio_darwin.go [new file with mode: 0644]
src/cmd/go/internal/robustio/robustio_flaky.go [new file with mode: 0644]
src/cmd/go/internal/robustio/robustio_other.go
src/cmd/go/internal/robustio/robustio_windows.go

index ace6e493cb4ee094157fe8a9414ea838bc795f32..ee2f3ba1bb8f7f956d88f57d328f7ee9abb18fa4 100644 (file)
@@ -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 (file)
index 0000000..99fd8eb
--- /dev/null
@@ -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 (file)
index 0000000..e57c8c7
--- /dev/null
@@ -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)
+       })
+}
index 56e6ad6d9c8f293372383797527339721c64e2e6..907b556858712ddf46d0812d76e3b6e18907b7bc 100644 (file)
@@ -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
 
index a3d94e566fe420340f93aa1d4d3da5c544e1c051..687dcb66f83d15d0be4c7d45f4652a236afb2ac2 100644 (file)
@@ -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 {