Skip to content

Commit 8bd7fb5

Browse files
rahulaitrahulait
authored
[improvement] : simplify rate limits as we now have only one limit for POST calls (#540)
* simplify rate limits as we now have only one limit for POST calls * remove Default POST limit values to simplify things
1 parent 1c49829 commit 8bd7fb5

File tree

4 files changed

+22
-70
lines changed

4 files changed

+22
-70
lines changed

‎controller/linodemachine_controller_helpers.go

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,21 +63,10 @@ var (
6363
errNoPublicIPv6SLAACAddrs = errors.New("no public SLAAC address set")
6464
)
6565

66-
func handleTooManyRequestsError(err error) (ctrl.Result, error) {
67-
newErr := linodego.NewError(err)
68-
if newErr.Response == nil {
69-
return ctrl.Result{RequeueAfter: reconciler.DefaultLinodeTooManyRequestsErrorRetryDelay}, nil
70-
}
71-
if newErr.Response.Request.Method != http.MethodPost {
72-
return ctrl.Result{RequeueAfter: reconciler.DefaultLinodeTooManyRequestsErrorRetryDelay}, nil
73-
}
74-
return ctrl.Result{RequeueAfter: reconciler.DefaultLinodeTooManyPOSTRequestsErrorRetryDelay}, nil
75-
}
76-
7766
func retryIfTransient(err error) (ctrl.Result, error) {
7867
if util.IsRetryableError(err) {
7968
if linodego.ErrHasStatus(err, http.StatusTooManyRequests) {
80-
return handleTooManyRequestsError(err)
69+
return ctrl.Result{RequeueAfter: reconciler.DefaultLinodeTooManyRequestsErrorRetryDelay}, nil
8170
}
8271
return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil
8372
}
@@ -705,11 +694,11 @@ func createInstance(ctx context.Context, logger logr.Logger, machineScope *scope
705694
defer ctr.Mu.Unlock()
706695

707696
if ctr.IsPOSTLimitReached() {
708-
logger.Info(fmt.Sprintf("Cannot make more POST requests as rate-limit is reached (%d per %v seconds). Waiting and retrying after %v seconds", reconciler.SecondaryPOSTRequestLimit, reconciler.SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay, reconciler.SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay))
697+
logger.Info(fmt.Sprintf("Cannot make more POST requests as rate-limit is reached. Waiting and retrying after %v seconds", ctr.RetryAfter()))
709698
return nil, ctr.RetryAfter(), util.ErrRateLimit
710699
}
711700

712701
machineScope.LinodeClient.OnAfterResponse(ctr.ApiResponseRatelimitCounter)
713702
inst, err := machineScope.LinodeClient.CreateInstance(ctx, *createOpts)
714-
return inst, time.Duration(reconciler.SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay.Seconds()), err
703+
return inst, time.Duration(reconciler.DefaultMachineControllerRetryDelay.Seconds()), err
715704
}

‎util/ratelimits.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import (
2424
"time"
2525

2626
"github.com/go-resty/resty/v2"
27-
28-
"github.com/linode/cluster-api-provider-linode/util/reconciler"
2927
)
3028

3129
// PostRequestCounter keeps track of rate limits for POST to /linode/instances
@@ -59,22 +57,12 @@ func (c *PostRequestCounter) ApiResponseRatelimitCounter(resp *resty.Response) e
5957
return err
6058
}
6159
c.RefreshTime = time.Unix(epochTime, 0)
62-
// We Add a negative number as secondary refresh time is smaller than refresh time
63-
secondaryRefreshTime := time.Unix(epochTime, 0).Add(reconciler.SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay * -1)
64-
65-
// TODO: remove when rate-limits are simplified
66-
currTime := time.Now()
67-
if c.ReqRemaining >= reconciler.SecondaryPOSTRequestLimit && currTime.Before(secondaryRefreshTime) {
68-
c.RefreshTime = secondaryRefreshTime
69-
}
7060
return nil
7161
}
7262

7363
// IsPOSTLimitReached checks whether POST limits have been reached.
7464
func (c *PostRequestCounter) IsPOSTLimitReached() bool {
75-
// TODO: Once linode API adjusts rate-limits, remove secondary rate limit and simplify accordingly
76-
// if we have made 5 requests (5 remaining) or 10 requests (0 remaining), then we want to wait until refresh time has passed for that window
77-
return time.Now().Before(c.RefreshTime) && (c.ReqRemaining == 0 || c.ReqRemaining == reconciler.SecondaryPOSTRequestLimit)
65+
return time.Now().Before(c.RefreshTime) && c.ReqRemaining == 0
7866
}
7967

8068
// RetryAfter returns how long to wait in seconds for rate-limit to reset
@@ -94,7 +82,9 @@ func GetPostReqCounter(tokenHash string) *PostRequestCounter {
9482
ctr, exists := postRequestCounters[tokenHash]
9583
if !exists {
9684
ctr = &PostRequestCounter{
97-
ReqRemaining: reconciler.DefaultPOSTRequestLimit,
85+
// Set remaining requests to a number greater than 0.
86+
// It gets updated to correct value once first POST request is made using the token.
87+
ReqRemaining: 1,
9888
RefreshTime: time.Time{},
9989
}
10090
postRequestCounters[tokenHash] = ctr

‎util/ratelimits_test.go

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import (
2424
"time"
2525

2626
"github.com/go-resty/resty/v2"
27-
28-
"github.com/linode/cluster-api-provider-linode/util/reconciler"
2927
)
3028

3129
func TestGetPostReqCounter(t *testing.T) {
@@ -40,22 +38,22 @@ func TestGetPostReqCounter(t *testing.T) {
4038
name: "provide hash which exists in map",
4139
tokenHash: "abcdef",
4240
want: &PostRequestCounter{
43-
ReqRemaining: 5,
41+
ReqRemaining: 4,
4442
RefreshTime: now.Add(-100 * time.Second),
4543
},
4644
},
4745
{
4846
name: "provide hash which doesn't exist",
4947
tokenHash: "uvwxyz",
5048
want: &PostRequestCounter{
51-
ReqRemaining: reconciler.DefaultPOSTRequestLimit,
49+
ReqRemaining: 1,
5250
RefreshTime: time.Time{},
5351
},
5452
},
5553
}
5654
for _, tt := range tests {
5755
postRequestCounters["abcdef"] = &PostRequestCounter{
58-
ReqRemaining: reconciler.SecondaryPOSTRequestLimit,
56+
ReqRemaining: 4,
5957
RefreshTime: now.Add(-100 * time.Second),
6058
}
6159
t.Run(tt.name, func(t *testing.T) {
@@ -78,19 +76,11 @@ func TestPostRequestCounter_IsPOSTLimitReached(t *testing.T) {
7876
{
7977
name: "not reached rate limits",
8078
fields: &PostRequestCounter{
81-
ReqRemaining: 7,
79+
ReqRemaining: 3,
8280
RefreshTime: now,
8381
},
8482
want: false,
8583
},
86-
{
87-
name: "reached token rate limit",
88-
fields: &PostRequestCounter{
89-
ReqRemaining: reconciler.SecondaryPOSTRequestLimit,
90-
RefreshTime: now.Add(100 * time.Second),
91-
},
92-
want: true,
93-
},
9484
{
9585
name: "reached account rate limits",
9686
fields: &PostRequestCounter{
@@ -99,14 +89,6 @@ func TestPostRequestCounter_IsPOSTLimitReached(t *testing.T) {
9989
},
10090
want: true,
10191
},
102-
{
103-
name: "refresh time smaller than current time",
104-
fields: &PostRequestCounter{
105-
ReqRemaining: reconciler.SecondaryPOSTRequestLimit,
106-
RefreshTime: now.Add(-100 * time.Second),
107-
},
108-
want: false,
109-
},
11092
{
11193
name: "refresh time smaller than current time",
11294
fields: &PostRequestCounter{
@@ -142,7 +124,7 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
142124
{
143125
name: "not a POST call",
144126
fields: &PostRequestCounter{
145-
ReqRemaining: 6,
127+
ReqRemaining: 4,
146128
RefreshTime: now,
147129
},
148130
args: &resty.Response{
@@ -155,7 +137,7 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
155137
{
156138
name: "endpoint different than /linode/instances",
157139
fields: &PostRequestCounter{
158-
ReqRemaining: 6,
140+
ReqRemaining: 4,
159141
RefreshTime: now,
160142
},
161143
args: &resty.Response{
@@ -169,7 +151,7 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
169151
{
170152
name: "no headers in response",
171153
fields: &PostRequestCounter{
172-
ReqRemaining: 6,
154+
ReqRemaining: 4,
173155
RefreshTime: now,
174156
},
175157
args: &resty.Response{
@@ -183,7 +165,7 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
183165
{
184166
name: "missing one value in response header",
185167
fields: &PostRequestCounter{
186-
ReqRemaining: 6,
168+
ReqRemaining: 4,
187169
RefreshTime: now,
188170
},
189171
args: &resty.Response{
@@ -192,15 +174,15 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
192174
URL: "/v4/linode/instances",
193175
},
194176
RawResponse: &http.Response{
195-
Header: http.Header{"X-Ratelimit-Remaining": []string{"10"}},
177+
Header: http.Header{"X-Ratelimit-Remaining": []string{"5"}},
196178
},
197179
},
198180
wantErr: true,
199181
},
200182
{
201183
name: "correct headers in response",
202184
fields: &PostRequestCounter{
203-
ReqRemaining: 6,
185+
ReqRemaining: 4,
204186
RefreshTime: now,
205187
},
206188
args: &resty.Response{
@@ -209,15 +191,15 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
209191
URL: "/v4/linode/instances",
210192
},
211193
RawResponse: &http.Response{
212-
Header: http.Header{"X-Ratelimit-Remaining": []string{"10"}, "X-Ratelimit-Reset": []string{"10"}},
194+
Header: http.Header{"X-Ratelimit-Remaining": []string{"5"}, "X-Ratelimit-Reset": []string{"10"}},
213195
},
214196
},
215197
wantErr: false,
216198
},
217199
{
218200
name: "correct headers in response",
219201
fields: &PostRequestCounter{
220-
ReqRemaining: 8,
202+
ReqRemaining: 4,
221203
RefreshTime: now,
222204
},
223205
args: &resty.Response{
@@ -226,7 +208,7 @@ func TestPostRequestCounter_ApiResponseRatelimitCounter(t *testing.T) {
226208
URL: "/v4/linode/instances",
227209
},
228210
RawResponse: &http.Response{
229-
Header: http.Header{"X-Ratelimit-Remaining": []string{"10"}, "X-Ratelimit-Reset": []string{strconv.Itoa(int(time.Now().Unix()) + 100)}},
211+
Header: http.Header{"X-Ratelimit-Remaining": []string{"4"}, "X-Ratelimit-Reset": []string{strconv.Itoa(int(time.Now().Unix()) + 100)}},
230212
},
231213
},
232214
wantErr: false,
@@ -257,15 +239,15 @@ func TestPostRequestCounter_RetryAfter(t *testing.T) {
257239
{
258240
name: "when current time is greater than refreshTime",
259241
fields: &PostRequestCounter{
260-
ReqRemaining: 7,
242+
ReqRemaining: 3,
261243
RefreshTime: currTime.Add(-100 * time.Second),
262244
},
263245
want: 0,
264246
},
265247
{
266248
name: "when refreshTime is not yet reached",
267249
fields: &PostRequestCounter{
268-
ReqRemaining: reconciler.SecondaryPOSTRequestLimit,
250+
ReqRemaining: 4,
269251
RefreshTime: currTime.Add(100 * time.Second),
270252
},
271253
want: 101 * time.Second,

‎util/reconciler/defaults.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,6 @@ const (
6565

6666
// DefaultDNSTTLSec is the default TTL used for DNS entries for api server loadbalancing
6767
DefaultDNSTTLSec = 30
68-
69-
// DefaultLinodeTooManyPOSTRequestsErrorRetryDelay is the default requeue delay if there is Linode API error for POST request. Currently, it is set to 10 requests per 30 seconds
70-
DefaultLinodeTooManyPOSTRequestsErrorRetryDelay = 30 * time.Second
71-
// SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay is the secondary requeue delay if there is Linode API error for POST request. Currently, it is set to 5 requests per 15 seconds
72-
SecondaryLinodeTooManyPOSTRequestsErrorRetryDelay = 15 * time.Second
73-
// DefaultPOSTRequestLimit is the default limit of how many POST requests can be made to /linode/instances endpoint in 30 seconds before rate-limit reset.
74-
DefaultPOSTRequestLimit = 10
75-
// SecondaryPOSTRequestLimit is the secondary limit of how many POST requests can be made to /linode/instances endpoint in 15 seconds before rate-limit kicks in.
76-
SecondaryPOSTRequestLimit = 5
7768
)
7869

7970
// DefaultedLoopTimeout will default the timeout if it is zero-valued.

0 commit comments

Comments
 (0)