Skip to content

Backend: Cache leaderboard endpoint responses to reduce slow DB query #5075

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.

Sign up for GitHub

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

Open
akanshajain231999 wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Backend: Cache leaderboard endpoint responses to reduce slow DB query #5075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
108bf0e
Backend: Cache leaderboard endpoint responses to reduce slow DB query
akanshajain231999 Apr 28, 2026
5510b67
Backend: Apply black formatting to leaderboard cache tests
akanshajain231999 Apr 28, 2026
61726c0
Merge branch 'master' into sentry-fix2
RishabhJain2018 May 5, 2026
f1e105a
Merge branch 'master' into sentry-fix2
RishabhJain2018 May 16, 2026
58c467e
Merge branch 'master' into sentry-fix2
RishabhJain2018 May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions apps/jobs/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
get_participant_model,
)
from django.conf import settings
from django.core.cache import cache
from django.core.files.base import ContentFile
from django.core.files.uploadedfile import SimpleUploadedFile
from django.db import IntegrityError, transaction
Expand Down Expand Up @@ -636,16 +637,32 @@ def leaderboard(request, challenge_phase_split_id):
)
challenge_obj = challenge_phase_split.challenge_phase.challenge
order_by = request.GET.get("order_by")
(
response_data,
http_status_code,
) = calculate_distinct_sorted_leaderboard_data(
request.user,
challenge_obj,
challenge_phase_split,
only_public_entries=True,
order_by=order_by,

is_host_bit = int(
bool(is_user_a_staff_or_host(request.user, challenge_obj.pk))
)
cache_key = "leaderboard:v1:{phase_split}:{order_by}:{is_host}".format(
phase_split=challenge_phase_split_id,
order_by=order_by or "default",
is_host=is_host_bit,
Comment on lines +644 to +647

@coderabbitai coderabbitai Bot May 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Django's Memcached cache backend, what key length/character restrictions apply, and when is InvalidCacheKey raised?

💡 Result:

In Django’s Memcached cache backend, the key restrictions and the error behavior are: 1) Key length and character restrictions (memcached-safe) - Maximum key length: keys longer than 250 characters are considered invalid for memcached by Django’s validation logic. [1][2] - Disallowed characters: Django treats keys containing whitespace or control characters (and also DEL/control chars in the range \x00-\x20 and \x7f) as invalid for memcached. [2][1] These checks are applied through Django’s common memcached key warning/validation helpers. Django’s docs explicitly state that memcached keys “does not allow cache keys longer than 250 characters or containing whitespace or control characters”. [2] 2) When InvalidCacheKey is raised - In Django’s memcached backend implementation (MemcachedCache), validate_key iterates over memcache_key_warnings(key) and raises InvalidCacheKey(warning) when any warning is produced. [3] - The warnings are produced specifically when the key violates the above rules (length > 250, or disallowed characters). [1] - Therefore, InvalidCacheKey is raised when you call cache operations on a memcached backend with a key that would violate those memcached constraints (length/whitespace/control chars). This happens during key preparation for backend calls via make_and_validate_key. [3][1] Concretely, calls like cache.get/add/set/touch on the memcached backend will raise InvalidCacheKey if make_and_validate_key(key) detects a memcached-incompatible key. [3][1]

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '620,680p' apps/jobs/views.py | cat -n

Repository: Cloud-CV/EvalAI

Length of output: 2554

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where cache backend is configured (look for memcached backend paths / env usage)
rg -n "memcached|django\.core\.cache\.backends\.memcached|CACHE_BACKEND|CACHES\s*=" . || true

# Also quickly list likely settings files
ls -la . | head -n 50
find . -maxdepth 3 -type f -iname "*settings*.py" | head -n 50

Repository: Cloud-CV/EvalAI

Length of output: 3870

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show memcached cache configuration
sed -n '280,360p' settings/common.py | cat -n

# Check if the leaderboard view handles InvalidCacheKey
sed -n '600,690p' apps/jobs/views.py | cat -n

# See if repo catches InvalidCacheKey anywhere
rg -n "InvalidCacheKey" . || true

Repository: Cloud-CV/EvalAI

Length of output: 7223

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "InvalidCacheKey" . || true

# Find the leaderboard calc function implementation and inspect order_by handling
rg -n "def calculate_distinct_sorted_leaderboard_data" -S apps || true
rg -n "calculate_distinct_sorted_leaderboard_data\\(" -S apps/jobs/views.py || true

# Narrow read around the function once located (best-effort by printing a few likely ranges)
python3 - <<'PY'
import subprocess, re
out = subprocess.check_output(["bash","-lc","rg -n \"def calculate_distinct_sorted_leaderboard_data\" -S apps || true"], text=True)
print(out.strip())
PY

Repository: Cloud-CV/EvalAI

Length of output: 491

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '280,460p' apps/jobs/utils.py | cat -n

rg -n "InvalidCacheKey" . || true

rg -n "order_by\\s*=\\s*|order_by\\s*in\\s*|whitelist|allowed.*order|default_order_by" apps/jobs/utils.py || true

sed -n '460,620p' apps/jobs/utils.py | cat -n

Repository: Cloud-CV/EvalAI

Length of output: 15869

Don’t embed raw order_by query param into the Memcached cache key

apps/jobs/views.py (leaderboard, lines 644-647) builds cache_key directly from request.GET.get("order_by") (with only a "default" fallback). Even though calculate_distinct_sorted_leaderboard_data() later whitelists order_by against leaderboard.schema["labels"] for the computation, the cache key itself is still composed from the raw input. With the Memcached cache backend, Django raises InvalidCacheKey for keys that are too long (>250 chars) or contain whitespace/control characters, and there’s no exception handling around cache.get() / cache.set(), so this can surface as a 500.

Proposed fix 
+import hashlib
@@
-    cache_key = "leaderboard:v1:{phase_split}:{order_by}:{is_host}".format(
+    order_by_key = order_by or "default"
+    safe_order_by_key = hashlib.sha256(
+        order_by_key.encode("utf-8")
+    ).hexdigest()[:16]
+    cache_key = "leaderboard:v1:{phase_split}:{order_by}:{is_host}".format(
         phase_split=challenge_phase_split_id,
-        order_by=order_by or "default",
+        order_by=safe_order_by_key,
         is_host=is_host_bit,
     )
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/jobs/views.py` around lines 644 - 647, The cache key for the leaderboard
view is built from raw request.GET.get("order_by") (variable cache_key in
leaderboard) which can contain long or invalid characters and trigger
InvalidCacheKey; before interpolating into cache_key, validate or normalize the
order_by value by checking it against the whitelist used in
calculate_distinct_sorted_leaderboard_data() (leaderboard.schema["labels"]) and
defaulting to "default" if not allowed, or else replace/encode it (e.g. URL-safe
base64 or a short hash) to guarantee no whitespace/control chars and length
<250; additionally wrap cache.get/cache.set calls in a try/except that catches
InvalidCacheKey (or Exception) to fall back to non-cached behavior so a bad key
cannot raise a 500.

)
cached = cache.get(cache_key)
if cached is not None:
response_data, http_status_code = cached
else:
(
response_data,
http_status_code,
) = calculate_distinct_sorted_leaderboard_data(
request.user,
challenge_obj,
challenge_phase_split,
only_public_entries=True,
order_by=order_by,
)
if http_status_code == status.HTTP_200_OK:
cache.set(cache_key, (response_data, http_status_code), 30)

# The response 400 will be returned if the leaderboard isn't public or
# `default_order_by` key is missing in leaderboard.
if http_status_code == status.HTTP_400_BAD_REQUEST:
Expand Down
100 changes: 100 additions & 0 deletions tests/unit/jobs/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@
LeaderboardData,
)
from django.contrib.auth.models import User
from django.core.cache import cache
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import override_settings
from django.urls import reverse_lazy
from django.utils import timezone
from hosts.models import ChallengeHost, ChallengeHostTeam
from jobs.models import Submission
from jobs.utils import calculate_distinct_sorted_leaderboard_data
from moto import mock_s3
from participants.models import Participant, ParticipantTeam
from rest_framework import status
Expand Down Expand Up @@ -2549,6 +2552,103 @@ def test_get_leaderboard_hides_scores_when_show_scores_on_leaderboard_false(
self.assertNotIn("filtering_error", entry)
self.assertIn("submission__participant_team__team_name", entry)

@override_settings(
CACHES={
"default": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
"LOCATION": "leaderboard-cache-test-1",
}
}
)
def test_get_leaderboard_uses_cache_on_subsequent_request(self):
cache.clear()
url = reverse_lazy(
"jobs:leaderboard",
kwargs={"challenge_phase_split_id": self.challenge_phase_split.id},
)
with mock.patch(
"jobs.views.calculate_distinct_sorted_leaderboard_data",
wraps=calculate_distinct_sorted_leaderboard_data,
) as wrapped:
first = self.client.get(url, {})
second = self.client.get(url, {})
self.assertEqual(first.status_code, status.HTTP_200_OK)
self.assertEqual(second.status_code, status.HTTP_200_OK)
self.assertEqual(wrapped.call_count, 1)
self.assertEqual(first.data["results"], second.data["results"])

@override_settings(
CACHES={
"default": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
"LOCATION": "leaderboard-cache-test-2",
}
}
)
def test_get_leaderboard_caches_separately_for_host_vs_non_host(self):
cache.clear()
url = reverse_lazy(
"jobs:leaderboard",
kwargs={"challenge_phase_split_id": self.challenge_phase_split.id},
)
with mock.patch(
"jobs.views.calculate_distinct_sorted_leaderboard_data",
wraps=calculate_distinct_sorted_leaderboard_data,
) as wrapped:
self.client.force_authenticate(user=self.user1)
self.client.get(url, {})
self.client.force_authenticate(user=self.user)
self.client.get(url, {})
self.assertEqual(wrapped.call_count, 2)

@override_settings(
CACHES={
"default": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
"LOCATION": "leaderboard-cache-test-3",
}
}
)
def test_get_leaderboard_caches_separately_for_different_order_by(self):
cache.clear()
url = reverse_lazy(
"jobs:leaderboard",
kwargs={"challenge_phase_split_id": self.challenge_phase_split.id},
)
with mock.patch(
"jobs.views.calculate_distinct_sorted_leaderboard_data",
wraps=calculate_distinct_sorted_leaderboard_data,
) as wrapped:
self.client.get(url, {})
self.client.get(url, {"order_by": "test-score"})
self.assertEqual(wrapped.call_count, 2)

@override_settings(
CACHES={
"default": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
"LOCATION": "leaderboard-cache-test-4",
}
}
)
def test_get_leaderboard_does_not_cache_400_response(self):
cache.clear()
url = reverse_lazy(
"jobs:leaderboard",
kwargs={
"challenge_phase_split_id": self.private_challenge_phase_split.id
},
)
with mock.patch(
"jobs.views.calculate_distinct_sorted_leaderboard_data",
wraps=calculate_distinct_sorted_leaderboard_data,
) as wrapped:
first = self.client.get(url, {})
second = self.client.get(url, {})
self.assertEqual(first.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(second.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(wrapped.call_count, 2)


class UpdateSubmissionTest(BaseAPITestClass):
def setUp(self):
Expand Down
Loading