Skip to content

Commit

Permalink
Set floor of one second for polling delay (#15905)
Browse files Browse the repository at this point in the history 
To avoid excessive polling.
Add 429 to list of retriable HTTP status codes.
  • Loading branch information
@jhendrixMSFT
jhendrixMSFT authored Oct 26, 2021
1 parent 9b1992f commit 7b31812
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 27 deletions.
1 change: 1 addition & 0 deletions sdk/azcore/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* Added string typdef `arm.Endpoint` to provide a hint toward expected ARM client endpoints
* `azcore.ClientOptions` contains common pipeline configuration settings
* Added support for multi-tenant authorization in `arm/runtime`
* Require one second minimum when calling `PollUntilDone()`

### Bug Fixes
* Fixed a potential panic when creating the default Transporter.
Expand Down
12 changes: 6 additions & 6 deletions sdk/azcore/arm/runtime/poller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestNewPollerAsync(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -147,7 +147,7 @@ func TestNewPollerBody(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -184,7 +184,7 @@ func TestNewPollerLoc(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestNewPollerInitialRetryAfter(t *testing.T) {
t.Fatalf("unexpected poller type %s", pt.String())
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -273,7 +273,7 @@ func TestNewPollerFailedWithError(t *testing.T) {
t.Fatalf("unexpected poller type %s", pt.String())
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err == nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -309,7 +309,7 @@ func TestNewPollerSuccessNoContent(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 4 additions & 2 deletions sdk/azcore/internal/pollers/poller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,11 @@ func (l *Poller) FinalResponse(ctx context.Context, respType interface{}) (*http
// PollUntilDone will handle the entire span of the polling operation until a terminal state is reached,
// then return the final HTTP response for the polling operation and unmarshal the content of the payload
// into the respType interface that is provided.
// freq - the time to wait between polling intervals if the endpoint doesn't send a Retry-After header.
// A good starting value is 30 seconds. Note that some resources might benefit from a different value.
// freq - the time to wait between intervals in absence of a Retry-After header. Minimum is one second.
func (l *Poller) PollUntilDone(ctx context.Context, freq time.Duration, respType interface{}) (*http.Response, error) {
if freq < time.Second {
return nil, errors.New("polling frequency minimum is one second")
}
start := time.Now()
logPollUntilDoneExit := func(v interface{}) {
log.Writef(log.EventLRO, "END PollUntilDone() for %T: %v, total time: %s", l.lro, v, time.Since(start))
Expand Down
15 changes: 11 additions & 4 deletions sdk/azcore/internal/pollers/poller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ func TestNewPoller(t *testing.T) {
t.Fatal("unexpected empty resume token")
}
resp, err = p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
if err == nil {
t.Fatal("unexpected nil error")
}
if resp != nil {
t.Fatal("expected nil response")
}
resp, err = p.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -167,7 +174,7 @@ func TestNewPollerWithFinalGET(t *testing.T) {
Shape string `json:"shape"`
}
var w widget
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, &w)
resp, err := p.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -198,7 +205,7 @@ func TestNewPollerFail1(t *testing.T) {
p := NewPoller(&fakePoller{Ep: srv.URL()}, firstResp, pl, func(*http.Response) error {
return errors.New("failed")
})
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
resp, err := p.PollUntilDone(context.Background(), time.Second, nil)
if err == nil {
t.Fatal("unexpected nil error")
} else if s := err.Error(); s != "failed" {
Expand All @@ -221,7 +228,7 @@ func TestNewPollerFail2(t *testing.T) {
p := NewPoller(&fakePoller{Ep: srv.URL()}, firstResp, pl, func(*http.Response) error {
return errors.New("failed")
})
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
resp, err := p.PollUntilDone(context.Background(), time.Second, nil)
if err == nil {
t.Fatal("unexpected nil error")
} else if s := err.Error(); s != "failed" {
Expand All @@ -244,7 +251,7 @@ func TestNewPollerError(t *testing.T) {
p := NewPoller(&fakePoller{Ep: srv.URL()}, firstResp, pl, func(*http.Response) error {
return errors.New("failed")
})
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
resp, err := p.PollUntilDone(context.Background(), time.Second, nil)
if err == nil {
t.Fatal("unexpected nil error")
} else if s := err.Error(); s != "fatal" {
Expand Down
1 change: 1 addition & 0 deletions sdk/azcore/runtime/policy_retry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func setDefaults(o *policy.RetryOptions) {
if o.StatusCodes == nil {
o.StatusCodes = []int{
http.StatusRequestTimeout, // 408
http.StatusTooManyRequests, // 429
http.StatusInternalServerError, // 500
http.StatusBadGateway, // 502
http.StatusServiceUnavailable, // 503
Expand Down
30 changes: 15 additions & 15 deletions sdk/azcore/runtime/poller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestLocPollerSimple(t *testing.T) {
if !closed() {
t.Fatal("initial response body wasn't closed")
}
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err := lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -141,7 +141,7 @@ func TestLocPollerWithWidget(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -178,7 +178,7 @@ func TestLocPollerCancelled(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err == nil {
t.Fatal("unexpected nil error")
}
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestLocPollerWithError(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err == nil {
t.Fatal("unexpected nil error")
}
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestLocPollerWithResumeToken(t *testing.T) {
if err != nil {
t.Fatal(err)
}
resp, err = lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err = lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand All @@ -295,7 +295,7 @@ func TestLocPollerWithTimeout(t *testing.T) {
srv, close := mock.NewServer()
srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted))
srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted))
srv.AppendResponse(mock.WithSlowResponse(2 * time.Second))
srv.AppendResponse(mock.WithSlowResponse(5 * time.Second))
defer close()

firstResp := &http.Response{
Expand All @@ -314,8 +314,8 @@ func TestLocPollerWithTimeout(t *testing.T) {
if !closed() {
t.Fatal("initial response body wasn't closed")
}
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
resp, err := lro.PollUntilDone(ctx, 5*time.Millisecond, nil)
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
resp, err := lro.PollUntilDone(ctx, time.Second, nil)
cancel()
if err == nil {
t.Fatal("unexpected nil error")
Expand Down Expand Up @@ -357,7 +357,7 @@ func TestOpPollerSimple(t *testing.T) {
if !closed() {
t.Fatal("initial response body wasn't closed")
}
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err := lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -401,7 +401,7 @@ func TestOpPollerWithWidgetPUT(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -449,7 +449,7 @@ func TestOpPollerWithWidgetPOSTLocation(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -495,7 +495,7 @@ func TestOpPollerWithWidgetPOST(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -544,7 +544,7 @@ func TestOpPollerWithWidgetResourceLocation(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -613,7 +613,7 @@ func TestOpPollerWithResumeToken(t *testing.T) {
if err != nil {
t.Fatal(err)
}
resp, err = lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err = lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -656,7 +656,7 @@ func TestNopPoller(t *testing.T) {
if resp != firstResp {
t.Fatal("unexpected response")
}
resp, err = lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err = lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit 7b31812

Please sign in to comment.