Skip to content

Commit

Permalink
Respect dial_timeout flag in rewrapper.
Browse files Browse the repository at this point in the history
This CL addresses a regression introduced in
[]
that makes rewrapper no longer respect the dial timeout. This likely
explains the elevated rate of build failures due to socket
unavailability in Android starting at 0.111.

Bug: b/297167920
Test: unit tests, manual tests with varying dial timeouts.
Change-Id: If13e000693b66b87f09146d6d9062da26b4bd672
GitOrigin-RevId: c03f4286d021c219b1da1a6f50ac1b3e570df629
  • Loading branch information
ramymedhat authored and copybara-github committed Aug 24, 2023
1 parent 3c7828f commit b1c25fe
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 10 deletions.
6 changes: 2 additions & 4 deletions cmd/rewrapper/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,14 @@ func main() {
log.Fatalf("No exec_strategy provided, must be one of %v", execStrategies)
}

ctx, cancel := context.WithTimeout(context.Background(), *dialTimeout)
defer cancel()
ctx := context.Background()
conn, err := ipc.DialContext(ctx, serverAddr)
if err != nil {
log.Fatalf("Fail to dial %s: %v", serverAddr, err)
}
defer conn.Close()

proxy := pb.NewCommandsClient(conn)
ctx = context.Background()
wd, err := os.Getwd()
if err != nil {
log.Fatalf("Failed to get current working directory: %v", err)
Expand Down Expand Up @@ -162,7 +160,7 @@ func main() {

// TODO (b/296409009): Add support for preserve true and download outputs false for downloading stubs.

resp, err := rewrapper.RunCommand(ctx, proxy, cmd, cOpts)
resp, err := rewrapper.RunCommand(ctx, *dialTimeout, proxy, cmd, cOpts)
if err != nil {
// Don't use log.Fatalf to avoid printing a stack trace.
log.Exitf("Command failed: %v", err)
Expand Down
2 changes: 2 additions & 0 deletions internal/pkg/rewrapper/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ go_test(
"@com_github_bazelbuild_remote_apis_sdks//go/pkg/command",
"@com_github_google_go_cmp//cmp",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//codes",
"@org_golang_google_grpc//status",
"@org_golang_google_protobuf//testing/protocmp",
],
)
13 changes: 8 additions & 5 deletions internal/pkg/rewrapper/rewrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package rewrapper

import (
"context"
"fmt"
"os"
"strings"
"time"
Expand All @@ -42,11 +43,9 @@ const (
)

var (
backoff = retry.ExponentialBackoff(1*time.Second, 15*time.Second, retry.Attempts(10))
// backoff has unlimited attempts because retry overall time is controlled by dialTimeout.
backoff = retry.ExponentialBackoff(1*time.Second, 15*time.Second, retry.UnlimitedAttempts)
shouldRetry = func(err error) bool {
if err == context.DeadlineExceeded {
return true
}
s, ok := status.FromError(err)
if !ok {
return false
Expand Down Expand Up @@ -103,13 +102,17 @@ type CommandOptions struct {
}

// RunCommand runs a command through the RE proxy.
func RunCommand(ctx context.Context, proxy Proxy, cmd []string, opts *CommandOptions) (*ppb.RunResponse, error) {
func RunCommand(ctx context.Context, dialTimeout time.Duration, proxy Proxy, cmd []string, opts *CommandOptions) (*ppb.RunResponse, error) {
req, err := createRequest(cmd, opts)
if err != nil {
return nil, err
}
var resp *ppb.RunResponse
st := time.Now()
err = retry.WithPolicy(ctx, shouldRetry, backoff, func() error {
if time.Since(st) > dialTimeout {
return fmt.Errorf("dial_timeout of %v expired before being able to connect to reproxy", dialTimeout)
}
resp, err = proxy.RunCommand(ctx, req)
return err
})
Expand Down
11 changes: 10 additions & 1 deletion internal/pkg/rewrapper/rewrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"github.com/bazelbuild/remote-apis-sdks/go/pkg/command"
"github.com/google/go-cmp/cmp"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/testing/protocmp"

ppb "team/foundry-x/re-client/api/proxy"
Expand Down Expand Up @@ -115,7 +117,7 @@ func TestRunCommand(t *testing.T) {
},
}
p := &proxyStub{}
if _, err := RunCommand(context.Background(), p, cmd, opts); err != nil {
if _, err := RunCommand(context.Background(), time.Hour, p, cmd, opts); err != nil {
t.Errorf("RunCommand(%v,%v) returned error: %v", cmd, opts, err)
}
strSliceCmp := protocmp.SortRepeated(func(a, b string) bool { return a < b })
Expand All @@ -124,6 +126,13 @@ func TestRunCommand(t *testing.T) {
}
}

func TestRunCommandTimeout(t *testing.T) {
p := &proxyStub{err: status.Error(codes.Unavailable, "error")}
if _, err := RunCommand(context.Background(), time.Second, p, []string{}, &CommandOptions{}); err == nil {
t.Fatalf("RunCommand returned no error, expecting dial timeout error")
}
}

func createRspFile(t *testing.T, contents []string) string {
tmpFile, err := os.CreateTemp(os.TempDir(), "")
if err != nil {
Expand Down

0 comments on commit b1c25fe

Please sign in to comment.