-
-
Notifications
You must be signed in to change notification settings - Fork 3k
feat: added 15 minutes of TTL for 1st page of all time leaderboard (@LuckySilver0021) #7447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
8ae76df
fb11861
26cf1a2
df7684c
f2d73ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,7 @@ import { MonkeyRequest } from "../types"; | |
| import { getFunbox, checkCompatibility } from "@monkeytype/funbox"; | ||
| import { tryCatch } from "@monkeytype/util/trycatch"; | ||
| import { getCachedConfiguration } from "../../init/configuration"; | ||
| import { allTimeLeaderboardCache } from "../../utils/all-time-leaderboard-cache"; | ||
|
|
||
| try { | ||
| if (!anticheatImplemented()) throw new Error("undefined"); | ||
|
|
@@ -534,6 +535,13 @@ export async function addResult( | |
| }, | ||
| dailyLeaderboardsConfig, | ||
| ); | ||
| try { | ||
| allTimeLeaderboardCache.clear(); | ||
| console.log("All-time leaderboard cache cleared"); | ||
| } catch (error) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how can this happen? |
||
| console.warn("Cache clear failed (non-critical):", error); | ||
| } | ||
|
|
||
| if ( | ||
| dailyLeaderboardRank >= 1 && | ||
| dailyLeaderboardRank <= 10 && | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ import { DBUser, getUsersCollection } from "./user"; | |
| import MonkeyError from "../utils/error"; | ||
| import { aggregateWithAcceptedConnections } from "./connections"; | ||
|
|
||
| import { allTimeLeaderboardCache } from "../utils/all-time-leaderboard-cache"; | ||
|
|
||
| export type DBLeaderboardEntry = LeaderboardEntry & { | ||
| _id: ObjectId; | ||
| }; | ||
|
|
@@ -46,6 +48,14 @@ export async function get( | |
| throw new MonkeyError(500, "Invalid page or pageSize"); | ||
| } | ||
|
|
||
| if (page === 0 && pageSize === 50 && uid === undefined) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets move the cache to the |
||
| const cached = allTimeLeaderboardCache.get({ mode, language }); | ||
| if (cached) { | ||
| console.log("✅ Cache HIT - leaderboards"); | ||
| return cached.data as DBLeaderboardEntry[]; | ||
| } | ||
| } | ||
|
|
||
| const skip = page * pageSize; | ||
| const limit = pageSize; | ||
|
|
||
|
|
@@ -83,12 +93,23 @@ export async function get( | |
| leaderboard = leaderboard.map((it) => omit(it, ["isPremium"])); | ||
| } | ||
|
|
||
| if (page === 0 && pageSize === 50 && uid === undefined) { | ||
| try { | ||
| allTimeLeaderboardCache.set( | ||
| { mode, language }, | ||
| leaderboard, | ||
| await getCount(mode, mode2, language), | ||
| ); | ||
| console.log(" Cache SET - leaderboards"); | ||
| } catch (error) { | ||
| console.warn("Cache set failed:", error); | ||
| } | ||
| } | ||
|
|
||
| return leaderboard; | ||
| } catch (e) { | ||
| // oxlint-disable-next-line no-unsafe-member-access | ||
| if (e.error === 175) { | ||
| if ((e as unknown as { error: number }).error === 175) { | ||
| //QueryPlanKilled, collection was removed during the query | ||
| return false; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reverting this will scream linting errors and it wont commit the code changes, I tried fixing it but I couldnt make it. as of now I will just update the pr except this part only becuase I couldnt commit and push the changes, further let me know how to deal with that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| throw e; | ||
| } | ||
|
|
@@ -162,10 +183,8 @@ export async function getRank( | |
| return results[0] ?? null; | ||
| } | ||
| } catch (e) { | ||
| // oxlint-disable-next-line no-unsafe-member-access | ||
| if (e.error === 175) { | ||
| if ((e as unknown as { error: number }).error === 175) { | ||
| //QueryPlanKilled, collection was removed during the query | ||
| return false; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert |
||
| } | ||
| throw e; | ||
| } | ||
|
|
@@ -393,8 +412,8 @@ async function createIndex( | |
| Logger.warning(`Index ${key} not matching, dropping and recreating...`); | ||
|
|
||
| const existingIndex = (await getUsersCollection().listIndexes().toArray()) | ||
| // oxlint-disable-next-line no-unsafe-member-access | ||
| .map((it) => it.name as string) | ||
|
|
||
| .map((it: unknown) => (it as { name: string }).name) | ||
| .find((it) => it.startsWith(key)); | ||
|
|
||
| if (existingIndex !== undefined && existingIndex !== null) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| type AllTimeCacheKey = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add mode2 to the cache key |
||
| mode: string; | ||
| language: string; | ||
| }; | ||
|
|
||
| type CacheEntry = { | ||
| data: unknown[]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keep the type |
||
| count: number; | ||
| timestamp: number; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't need a timestamp and only clear using
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the timestamp is removed then how will we we keep track of the 15 minutes?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check the discussion in the original pr: #7415 (comment) We know when the leaderboard is refreshed and can invalidate the cache at this time.
|
||
| }; | ||
|
|
||
| class AllTimeLeaderboardCache { | ||
| private cache = new Map<string, CacheEntry>(); | ||
| private readonly TTL = 900_000; // == 15 minutes of TTL | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove |
||
|
|
||
| private getKey({ mode, language }: AllTimeCacheKey): string { | ||
| return `alltime-lb:${mode}:${language}`; | ||
| } | ||
|
|
||
| get(key: AllTimeCacheKey): CacheEntry | null { | ||
| const cacheKey = this.getKey(key); | ||
| const entry = this.cache.get(cacheKey); | ||
|
|
||
| if (!entry || Date.now() - entry.timestamp > this.TTL) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove |
||
| if (entry) this.cache.delete(cacheKey); | ||
| return null; | ||
| } | ||
| return entry; | ||
| } | ||
|
|
||
| set(key: AllTimeCacheKey, data: unknown[], count: number): void { | ||
| this.cache.set(this.getKey(key), { data, count, timestamp: Date.now() }); | ||
| } | ||
|
|
||
| clear(): void { | ||
| this.cache.clear(); | ||
| } | ||
| } | ||
|
|
||
| export const allTimeLeaderboardCache = new AllTimeLeaderboardCache(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { getConnection } from "../init/redis"; | ||
|
|
||
| const CACHE_PREFIX = "cache:"; | ||
| const TTL = 300; // == 5 minutes | ||
|
|
||
| export async function getCached<T>(key: string): Promise<T | null> { | ||
| const redis = getConnection(); | ||
| if (!redis) return null; | ||
|
|
||
| try { | ||
| const data = await redis.get(`${CACHE_PREFIX}${key}`); | ||
| if (data === null || data === undefined || data === "") return null; | ||
| return JSON.parse(data) as T; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export async function setCached<T>(key: string, data: T): Promise<void> { | ||
| const redis = getConnection(); | ||
| if (!redis) return; | ||
|
|
||
| try { | ||
| await redis.setex(`${CACHE_PREFIX}${key}`, TTL, JSON.stringify(data)); | ||
| } catch (err) { | ||
| console.error("Cache set failed:", err); | ||
| } | ||
| } | ||
|
|
||
| export async function invalidateUserCache(userId: string): Promise<void> { | ||
| const redis = getConnection(); | ||
| if (!redis) return; | ||
|
|
||
| try { | ||
| const keys = await redis.keys(`${CACHE_PREFIX}user:profile:${userId}*`); | ||
| if (keys.length > 0) { | ||
| await redis.del(keys); | ||
| } | ||
| } catch (err) { | ||
| console.error("Cache invalidation failed:", err); | ||
| } | ||
| } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not the correct place, this is the dailyLeaderboard.