Skip to content

Commit b419a88

Browse files
committed
added retryAfter calculations to slidingWindowLog with tests
1 parent 9994ca0 commit b419a88

File tree

3 files changed

+112
-44
lines changed

3 files changed

+112
-44
lines changed

src/@types/rateLimit.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export interface RateLimiter {
1616
export interface RateLimiterResponse {
1717
success: boolean;
1818
tokens: number;
19+
retryAfter?: number;
1920
}
2021

2122
export interface RedisBucket {

src/rateLimiters/slidingWindowLog.ts

Lines changed: 64 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -38,35 +38,35 @@ class SlidingWindowLog implements RateLimiter {
3838
// TODO: Define lua script for server side computation
3939
// while x.timestamp + window_size < timestamp lpop
4040
// //https://stackoverflow.com/questions/35677682/filtering-deleting-items-from-a-redis-set
41-
this.client.defineCommand('popWindow', {
42-
// 2 value timestamp and complexity of this request
43-
lua: `
44-
local totalComplexity = 0 -- complexity of active requests
45-
local expiredMembers = 0 -- number of requests to remove
46-
local key = keys[1] -- uuid
47-
local current_time = keys[2]
48-
49-
for index, value in next, redis.call(key, ????) do
50-
-- string comparisson of timestamps
51-
if .... then
52-
53-
else
54-
totalComplexity += ????
55-
end
56-
end
57-
58-
redis.call(pop, ???)
59-
60-
if total_complexity < window_size then
61-
then
62-
end
63-
return {
64-
65-
}
66-
`,
67-
numberOfKeys: 3, // uuid
68-
readOnly: true,
69-
});
41+
// this.client.defineCommand('popWindow', {
42+
// // 2 value timestamp and complexity of this request
43+
// lua: `
44+
// local totalComplexity = 0 -- complexity of active requests
45+
// local expiredMembers = 0 -- number of requests to remove
46+
// local key = keys[1] -- uuid
47+
// local current_time = keys[2]
48+
49+
// for index, value in next, redis.call(key, ????) do
50+
// -- string comparisson of timestamps
51+
// if .... then
52+
53+
// else
54+
// totalComplexity += ????
55+
// end
56+
// end
57+
58+
// redis.call(pop, ???)
59+
60+
// if total_complexity < window_size then
61+
// then
62+
// end
63+
// return {
64+
65+
// }
66+
// `,
67+
// numberOfKeys: 3, // uuid
68+
// readOnly: true,
69+
// });
7070
}
7171

7272
/**
@@ -83,7 +83,6 @@ class SlidingWindowLog implements RateLimiter {
8383
): Promise<RateLimiterResponse> {
8484
// set the expiry of key-value pairs in the cache to 24 hours
8585
const keyExpiry = 86400000; // TODO: Make this a global for consistency across each algo.
86-
// if (tokens > this.capacity) return { success: false, tokens: this.capacity };
8786

8887
// Each user's log is represented by a redis list with a score = request timestamp
8988
// and a value equal to the complexity
@@ -92,21 +91,41 @@ class SlidingWindowLog implements RateLimiter {
9291
// Get the log from redis
9392
let requestLog: RedisLog = JSON.parse((await this.client.get(uuid)) || '[]');
9493

95-
// if (requestLog.length === 0) {
96-
// if (tokens > 0) requestLog.push({ timestamp, tokens });
97-
// // return { success: true, tokens: this.capacity - tokens };
98-
// }
94+
// Iterate through the list in reverse and count active tokens
95+
// This allows us to track the threshold for when this request would be allowed if it is blocked
96+
// Stop at the first timestamp that's expired and cut the rest.
97+
9998
const cutoff = timestamp - this.windowSize;
10099
let tokensInLog = 0;
101-
requestLog = requestLog.filter((bucket: RedisBucket) => {
102-
if (bucket.timestamp > cutoff) {
103-
// get complexity sum of active requests
104-
tokensInLog += bucket.tokens;
105-
return true;
100+
let cutoffIndex = 0; // index of oldest active request
101+
// TODO: Provide a timestamp for when the request will succeeed.
102+
// Compute time between response and this timestamp later on
103+
// FIXME: What should this be if the complexity is too big?
104+
let retryIndex = requestLog.length; // time the user must wait before a request can be allowed.
105+
106+
for (let index = requestLog.length - 1; index >= 0; index--) {
107+
if (cutoff >= requestLog[index].timestamp) {
108+
cutoffIndex = index + 1;
109+
break;
110+
} else {
111+
// the request is active
112+
tokensInLog += requestLog[index].tokens;
113+
if (this.capacity - tokensInLog >= tokens) {
114+
// the log is able to accept this request
115+
retryIndex = index;
116+
}
106117
}
107-
// drop expired requests
108-
return false;
109-
});
118+
}
119+
120+
let retryAfter: number;
121+
if (tokens > this.capacity) retryAfter = Infinity;
122+
// need the request before retryIndex
123+
else if (retryIndex > 0)
124+
retryAfter = this.windowSize + requestLog[retryIndex - 1].timestamp;
125+
else retryAfter = 0; // request is allowed
126+
127+
// Conditional check to avoid unecessary slice
128+
if (cutoffIndex > 0) requestLog = requestLog.slice(cutoffIndex);
110129

111130
// allow/disallow current request
112131
if (tokensInLog + tokens <= this.capacity) {
@@ -116,8 +135,10 @@ class SlidingWindowLog implements RateLimiter {
116135
tokensInLog += tokens;
117136
return { success: true, tokens: this.capacity - tokensInLog };
118137
}
138+
119139
await this.client.setex(uuid, keyExpiry, JSON.stringify(requestLog));
120-
return { success: false, tokens: this.capacity - tokensInLog };
140+
141+
return { success: false, tokens: this.capacity - tokensInLog, retryAfter };
121142
}
122143

123144
/**

test/rateLimiters/slidingWindowLog.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,11 @@ describe('SlidingWindowLog Rate Limiter', () => {
198198
]);
199199

200200
// Check the received response
201-
const expectedResponse: RateLimiterResponse = { tokens: CAPACITY, success: false };
201+
const expectedResponse: RateLimiterResponse = {
202+
tokens: CAPACITY,
203+
success: false,
204+
retryAfter: Infinity,
205+
};
202206
expect(user1Response).toEqual(expectedResponse);
203207
expect(user2Response).toEqual(expectedResponse);
204208
expect(user3Response).toEqual(expectedResponse);
@@ -421,6 +425,48 @@ describe('SlidingWindowLog Rate Limiter', () => {
421425
});
422426
});
423427

428+
describe('returns "retryAfter" if a request fails and', () => {
429+
/**
430+
* Strategy
431+
* Check where limitint request is at either end of log and in the middle
432+
* Infinity if > capacity (handled above)
433+
* doesn't appear if success (handled above)
434+
* */
435+
beforeEach(() => {
436+
timestamp = 1000;
437+
});
438+
439+
test('the limiting request was is at the beginning of the log', async () => {
440+
const requestLog = [
441+
{ timestamp, tokens: 9 }, // limiting request
442+
{ timestamp: timestamp + 100, tokens: 1 }, // newer request
443+
];
444+
await setLogInClient(client, user1, requestLog);
445+
const { retryAfter } = await limiter.processRequest(user1, timestamp + 200, 9);
446+
expect(retryAfter).toBe(timestamp + WINDOW_SIZE);
447+
});
448+
449+
test('the limiting request was is at the end of the log', async () => {
450+
const requestLog = [
451+
{ timestamp, tokens: 1 }, // older request
452+
{ timestamp: timestamp + 100, tokens: 9 }, // limiting request
453+
];
454+
await setLogInClient(client, user1, requestLog);
455+
const { retryAfter } = await limiter.processRequest(user1, timestamp + 200, 9);
456+
expect(retryAfter).toBe(timestamp + 100 + WINDOW_SIZE);
457+
});
458+
459+
test('the limiting request was is the middle of the log', async () => {
460+
const requestLog = [
461+
{ timestamp, tokens: 1 }, // older request
462+
{ timestamp: timestamp + 100, tokens: 8 }, // limiting request
463+
{ timestamp: timestamp + 200, tokens: 1 }, // newer request
464+
];
465+
await setLogInClient(client, user1, requestLog);
466+
const { retryAfter } = await limiter.processRequest(user1, timestamp + 200, 9);
467+
expect(retryAfter).toBe(timestamp + 100 + WINDOW_SIZE);
468+
});
469+
});
424470
xtest('users have their own logs', async () => {
425471
const requested = 6;
426472
const user3Tokens = 8;

0 commit comments

Comments
 (0)