Skip to content

Commit

Permalink
Remove setting the deadline based on the context (#12)
Browse files Browse the repository at this point in the history
  • Loading branch information
prep authored Aug 14, 2020
1 parent 5408f30 commit 1d0e426
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 107 deletions.
24 changes: 2 additions & 22 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,32 +84,12 @@ func (conn *Conn) String() string {
return conn.URI + " (local=" + conn.conn.LocalAddr().String() + ")"
}

// deadline returns the deadline of a single request and response.
func (conn *Conn) deadline(ctx context.Context) time.Time {
deadline, ok := ctx.Deadline()

// If no connection timeout has been configured, simply return the context
// deadline which could be set or not.
if conn.config.ConnTimeout == 0 {
return deadline
}

// If no context deadline was set or if the configured connection timeout
// will hit sooner, return the connection timeout.
timeout := time.Now().Add(conn.config.ConnTimeout)
if !ok || timeout.Before(deadline) {
return timeout
}

return deadline
}

func (conn *Conn) command(ctx context.Context, format string, params ...interface{}) (uint64, []byte, error) {
// Write a command and read the response.
id, body, err := func() (uint64, []byte, error) {
// Detect network problems early by setting a deadline.
if deadline := conn.deadline(ctx); !deadline.IsZero() {
if err := conn.conn.SetDeadline(deadline); err != nil {
if conn.config.ConnTimeout != 0 {
if err := conn.conn.SetDeadline(time.Now().Add(conn.config.ConnTimeout)); err != nil {
return 0, nil, err
}

Expand Down
85 changes: 0 additions & 85 deletions conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,88 +612,3 @@ func TestConn(t *testing.T) {
})
})
}

func TestDeadline(t *testing.T) {
conn := &Conn{}

testCases := []struct {
Name string
ConnTimeout time.Duration
ContextTimeout time.Duration
}{
{
Name: "WithoutAnyTimeoutOrContext",
},
{
Name: "WithTimeout",
ConnTimeout: 5 * time.Second,
},
{
Name: "WithContext",
ContextTimeout: 5 * time.Second,
},
{
Name: "WithSoonerTimeout",
ConnTimeout: 3 * time.Second,
ContextTimeout: 5 * time.Second,
},
{
Name: "WithSoonerContext",
ConnTimeout: 5 * time.Second,
ContextTimeout: 3 * time.Second,
},
}

for i, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
conn.config.ConnTimeout = testCase.ConnTimeout

// Create a context which might be a context with a timeout.
ctx := context.Background()
if testCase.ContextTimeout != 0 {
var cancel func()
ctx, cancel = context.WithTimeout(ctx, testCase.ContextTimeout)
defer cancel()
}

// Determine the expected value.
expect := func() time.Duration {
switch {
// If both are set, pick the one with the lowest value.
case testCase.ConnTimeout != 0 && testCase.ContextTimeout != 0:
if testCase.ConnTimeout < testCase.ContextTimeout {
return testCase.ConnTimeout
}
return testCase.ContextTimeout

// If only one is set and timeout is not 0, use that.
case testCase.ConnTimeout != 0:
return testCase.ConnTimeout

// Otherwise, use the context which could be either set or not.
default:
return testCase.ContextTimeout
}
}()

// Mark the expected deadline, which should be very close to the
// returned deadline.
mark := time.Now().Add(expect)

deadline := conn.deadline(ctx)
switch {
// Validate if the deadline is zero.
case expect == 0:
if !deadline.IsZero() {
t.Fatalf("Expected testcase %d to have no deadline, but got %s", i, deadline)
}

// Validate if the deadline is within a reasonable range.
case !mark.Add(-50 * time.Millisecond).Before(deadline):
t.Fatalf("Expected testcase %d deadline to be around %s", i, expect)
case !mark.Add(50 * time.Millisecond).After(deadline):
t.Fatalf("Expected testcase %d deadline to be around %s", i, expect)
}
})
}
}

0 comments on commit 1d0e426

Please sign in to comment.