feat: make edge use token's cache #6893
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| apiToken, | ||
| contextFrom(req), | ||
| ) | ||
| ? await apiTokenService.getUserForToken(apiToken) |
There was a problem hiding this comment.
Fully rolled out, no need for flag context
| expect( | ||
| await apiTokenService.getUserForToken(invalidToken), | ||
| await apiTokenService.getTokenWithCache(invalidToken), |
There was a problem hiding this comment.
Reducing the scope of the test to just fetching the token
| secret: string, | ||
| flagContext?: IFlagContext, // temporarily added, expected from the middleware | ||
| ): Promise<IApiUser | undefined> { | ||
| async getTokenWithCache(secret: string): Promise<IApiToken | undefined> { |
There was a problem hiding this comment.
This method is extracted from getUserForToken it was moved up to be close to getToken method above
| const validatedTokens: EdgeTokenSchema[] = []; | ||
| for (const token of tokens) { | ||
| const found = | ||
| await this.apiTokenService.getTokenWithCache(token); | ||
| if (found) { | ||
| validatedTokens.push({ | ||
| token: token, | ||
| type: found.type, | ||
| projects: found.projects, | ||
| }); | ||
| } |
There was a problem hiding this comment.
New behavior checking the tokens one by one, hitting the cache unless one is not present. Invalid tokens are also cached
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -140,10 +147,12 @@ export class ApiTokenService { | |||
| // prevent querying the same invalid secret multiple times. Expire after 5 minutes | |||
| this.queryAfter.set(secret, addMinutes(new Date(), 5)); | |||
|
|
|||
| const stopCacheTimer = this.timer('getTokenWithCache.query'); | |||
There was a problem hiding this comment.
This should help answer: how many times do we hit the DB when validating api tokens?
| } | ||
|
|
||
| async getValidTokens(tokens: string[]): Promise<ValidatedEdgeTokensSchema> { | ||
| if (this.flagResolver.isEnabled('checkEdgeValidTokensFromCache')) { | ||
| const stopTimer = this.timer('validateTokensWithCache'); |
There was a problem hiding this comment.
This should help answer which one is better in terms of time
About the changes
This PR removes the feature flag
queryMissingTokensthat was fully rolled out.It introduces a new way of checking edgeValidTokens controlled by the flag
checkEdgeValidTokensFromCachethat relies in the cached data but hits the DB if needed.The assumption is that most of the times edge will find tokens in the cache, except for a few cases in which a new token is queried. From all tokens we expect at most one to hit the DB and in this case querying a single token should be better than querying all the tokens.