From e49977858f20dbcbe553f8ac6cf65e2e736ac616 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Thu, 27 Apr 2023 09:06:12 +0200 Subject: [PATCH] Simplify API (#31) --- examples_test.go | 10 +++++----- hedged.go | 46 ++++++++------------------------------------ hedged_bench_test.go | 2 +- hedged_test.go | 28 +++++++++++++-------------- 4 files changed, 28 insertions(+), 58 deletions(-) diff --git a/examples_test.go b/examples_test.go index 4277f7e..7b1181d 100644 --- a/examples_test.go +++ b/examples_test.go @@ -21,7 +21,7 @@ func ExampleClient() { timeout := 10 * time.Millisecond upto := 7 client := &http.Client{Timeout: time.Second} - hedged, err := hedgedhttp.NewClient(timeout, upto, client) + hedged, _, err := hedgedhttp.NewClient(timeout, upto, client) if err != nil { panic(err) } @@ -48,7 +48,7 @@ func ExampleRoundTripper() { timeout := 10 * time.Millisecond upto := 7 transport := http.DefaultTransport - hedged, stats, err := hedgedhttp.NewRoundTripperAndStats(timeout, upto, transport) + hedged, stats, err := hedgedhttp.NewRoundTripper(timeout, upto, transport) if err != nil { panic(err) } @@ -78,7 +78,7 @@ func Example_instrumented() { Transport: http.DefaultTransport, } - _, err := hedgedhttp.NewRoundTripper(time.Millisecond, 3, transport) + _, _, err := hedgedhttp.NewRoundTripper(time.Millisecond, 3, transport) if err != nil { panic(err) } @@ -109,7 +109,7 @@ func Example_ratelimited() { Limiter: &RandomRateLimiter{}, } - _, err := hedgedhttp.NewRoundTripper(time.Millisecond, 3, transport) + _, _, err := hedgedhttp.NewRoundTripper(time.Millisecond, 3, transport) if err != nil { panic(err) } @@ -153,7 +153,7 @@ func ExampleMultiTransport() { Hedged: &http.Transport{MaxIdleConns: 30}, // just an example } - _, err := hedgedhttp.NewRoundTripper(time.Millisecond, 3, transport) + _, _, err := hedgedhttp.NewRoundTripper(time.Millisecond, 3, transport) if err != nil { panic(err) } diff --git a/hedged.go b/hedged.go index 56d65b0..1bf8bf6 100644 --- a/hedged.go +++ b/hedged.go @@ -13,58 +13,36 @@ import ( const infiniteTimeout = 30 * 24 * time.Hour // domain specific infinite // NewClient returns a new http.Client which implements hedged requests pattern. -// Given Client starts a new request after a timeout from previous request. -// Starts no more than upto requests. -func NewClient(timeout time.Duration, upto int, client *http.Client) (*http.Client, error) { - newClient, _, err := NewClientAndStats(timeout, upto, client) - if err != nil { - return nil, err - } - return newClient, nil -} - -// NewClientAndStats returns a new http.Client which implements hedged requests pattern // And Stats object that can be queried to obtain client's metrics. // Given Client starts a new request after a timeout from previous request. // Starts no more than upto requests. -func NewClientAndStats(timeout time.Duration, upto int, client *http.Client) (*http.Client, *Stats, error) { +func NewClient(timeout time.Duration, upto int, client *http.Client) (*http.Client, *Stats, error) { if client == nil { client = &http.Client{ Timeout: 5 * time.Second, } } - newTransport, metrics, err := NewRoundTripperAndStats(timeout, upto, client.Transport) + newTransport, stats, err := NewRoundTripper(timeout, upto, client.Transport) if err != nil { return nil, nil, err } client.Transport = newTransport - return client, metrics, nil + return client, stats, nil } // NewRoundTripper returns a new http.RoundTripper which implements hedged requests pattern. -// Given RoundTripper starts a new request after a timeout from previous request. -// Starts no more than upto requests. -func NewRoundTripper(timeout time.Duration, upto int, rt http.RoundTripper) (http.RoundTripper, error) { - newRT, _, err := NewRoundTripperAndStats(timeout, upto, rt) - if err != nil { - return nil, err - } - return newRT, nil -} - -// NewRoundTripperAndStats returns a new http.RoundTripper which implements hedged requests pattern // And Stats object that can be queried to obtain client's metrics. // Given RoundTripper starts a new request after a timeout from previous request. // Starts no more than upto requests. -func NewRoundTripperAndStats(timeout time.Duration, upto int, rt http.RoundTripper) (http.RoundTripper, *Stats, error) { +func NewRoundTripper(timeout time.Duration, upto int, rt http.RoundTripper) (http.RoundTripper, *Stats, error) { switch { case timeout < 0: - return nil, nil, errors.New("hedgedhttp: timeout cannot be negative") + return nil, nil, errors.New("timeout cannot be negative") case upto < 1: - return nil, nil, errors.New("hedgedhttp: upto must be greater than 0") + return nil, nil, errors.New("upto must be greater than 0") } if rt == nil { @@ -233,16 +211,11 @@ func runInPool(task func()) { // accumulate errors in cases and return them as a single "error". // Inspired by https://github.com/hashicorp/go-multierror type MultiError struct { - Errors []error - ErrorFormatFn ErrorFormatFunc + Errors []error } func (e *MultiError) Error() string { - fn := e.ErrorFormatFn - if fn == nil { - fn = listFormatFunc - } - return fn(e.Errors) + return listFormatFunc(e.Errors) } func (e *MultiError) String() string { @@ -259,9 +232,6 @@ func (e *MultiError) ErrorOrNil() error { } } -// ErrorFormatFunc is called by MultiError to return the list of errors as a string. -type ErrorFormatFunc func([]error) string - func listFormatFunc(es []error) string { if len(es) == 1 { return fmt.Sprintf("1 error occurred:\n\t* %s\n\n", es[0]) diff --git a/hedged_bench_test.go b/hedged_bench_test.go index 25e2f42..9598221 100644 --- a/hedged_bench_test.go +++ b/hedged_bench_test.go @@ -67,7 +67,7 @@ func BenchmarkHedgedRequest(b *testing.B) { errors := uint64(0) var snapshot atomic.Value - hedgedTarget, metrics, err := hedgedhttp.NewRoundTripperAndStats(10*time.Nanosecond, 10, target) + hedgedTarget, metrics, err := hedgedhttp.NewRoundTripper(10*time.Nanosecond, 10, target) if err != nil { b.Fatalf("want nil, got %s", err) } diff --git a/hedged_test.go b/hedged_test.go index e2673b9..e2d0796 100644 --- a/hedged_test.go +++ b/hedged_test.go @@ -17,22 +17,22 @@ import ( ) func TestValidateInput(t *testing.T) { - _, _, err := hedgedhttp.NewClientAndStats(-time.Second, 0, nil) + _, _, err := hedgedhttp.NewClient(-time.Second, 0, nil) if err == nil { t.Fatalf("want err, got nil") } - _, _, err = hedgedhttp.NewClientAndStats(time.Second, -1, nil) + _, _, err = hedgedhttp.NewClient(time.Second, -1, nil) if err == nil { t.Fatalf("want err, got nil") } - _, _, err = hedgedhttp.NewClientAndStats(time.Second, 0, nil) + _, _, err = hedgedhttp.NewClient(time.Second, 0, nil) if err == nil { t.Fatalf("want err, got nil") } - _, err = hedgedhttp.NewRoundTripper(time.Second, 0, nil) + _, _, err = hedgedhttp.NewRoundTripper(time.Second, 0, nil) if err == nil { t.Fatalf("want err, got nil") } @@ -52,7 +52,7 @@ func TestUpto(t *testing.T) { } const upto = 7 - client, err := hedgedhttp.NewClient(10*time.Millisecond, upto, nil) + client, _, err := hedgedhttp.NewClient(10*time.Millisecond, upto, nil) if err != nil { t.Fatal(err) } @@ -82,7 +82,7 @@ func TestUptoWithInstrumentation(t *testing.T) { } const upto = 7 - client, metrics, err := hedgedhttp.NewClientAndStats(10*time.Millisecond, upto, nil) + client, metrics, err := hedgedhttp.NewClient(10*time.Millisecond, upto, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -131,7 +131,7 @@ func TestNoTimeout(t *testing.T) { const upto = 10 - client, metrics, err := hedgedhttp.NewClientAndStats(0, upto, nil) + client, metrics, err := hedgedhttp.NewClient(0, upto, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -174,7 +174,7 @@ func TestFirstIsOK(t *testing.T) { t.Fatal(err) } - client, metrics, err := hedgedhttp.NewClientAndStats(10*time.Millisecond, 10, nil) + client, metrics, err := hedgedhttp.NewClient(10*time.Millisecond, 10, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -223,7 +223,7 @@ func TestBestResponse(t *testing.T) { if err != nil { t.Fatal(err) } - client, metrics, err := hedgedhttp.NewClientAndStats(10*time.Millisecond, 5, nil) + client, metrics, err := hedgedhttp.NewClient(10*time.Millisecond, 5, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -284,7 +284,7 @@ func TestGetSuccessEvenWithErrorsPresent(t *testing.T) { t.Fatal(err) } - client, metrics, err := hedgedhttp.NewClientAndStats(10*time.Millisecond, int(upto), nil) + client, metrics, err := hedgedhttp.NewClient(10*time.Millisecond, int(upto), nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -341,7 +341,7 @@ func TestGetFailureAfterAllRetries(t *testing.T) { t.Fatal(err) } - client, metrics, err := hedgedhttp.NewClientAndStats(time.Millisecond, upto, nil) + client, metrics, err := hedgedhttp.NewClient(time.Millisecond, upto, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -396,7 +396,7 @@ func TestHangAllExceptLast(t *testing.T) { t.Fatal(err) } - client, metrics, err := hedgedhttp.NewClientAndStats(10*time.Millisecond, upto, nil) + client, metrics, err := hedgedhttp.NewClient(10*time.Millisecond, upto, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -448,7 +448,7 @@ func TestCancelByClient(t *testing.T) { } upto := 5 - client, metrics, err := hedgedhttp.NewClientAndStats(10*time.Millisecond, upto, nil) + client, metrics, err := hedgedhttp.NewClient(10*time.Millisecond, upto, nil) if err != nil { t.Fatalf("want nil, got %s", err) } @@ -502,7 +502,7 @@ func TestIsHedged(t *testing.T) { } const upto = 7 - client, err := hedgedhttp.NewRoundTripper(10*time.Millisecond, upto, rt) + client, _, err := hedgedhttp.NewRoundTripper(10*time.Millisecond, upto, rt) if err != nil { t.Fatal(err) }