-
Notifications
You must be signed in to change notification settings - Fork 232
Caching Manage Submissions Page #2276
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
Conversation
…fter new submission.
…speed-investigation
📝 Walkthrough""" WalkthroughThe changes update two controllers to handle caching for submission data. In the assessment controller’s handin method, cache entries for submission IDs and the corresponding mapping are cleared when a new submission is processed. In the submissions controller, the index method now retrieves submission IDs and the mapping from a cache (stored for one day) instead of querying the database directly, while the new method additionally clears these caches upon creating a new submission. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Cache
participant DB
User->>Controller: Create new submission
Controller->>Cache: Clear cache entries ["submission_ids", assessment.id] and ["submissions_to_cud", assessment.id]
Controller->>DB: Process submission
User->>Controller: Request submissions list
Controller->>Cache: Check for submission_ids cache
alt Cache Hit
Cache-->>Controller: Return cached submission IDs
Controller->>DB: Query submissions using cached IDs
else Cache Miss
Controller->>DB: Query submissions directly
DB-->>Controller: Return submission data
Controller->>Cache: Cache submission IDs (1 day)
Controller->>Cache: Cache submissions_to_cud mapping (1 day)
end
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/controllers/assessment/handin.rb(1 hunks)app/controllers/submissions_controller.rb(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
app/controllers/assessment/handin.rb (1)
30-33: Good implementation of cache clearance when submissions are made.The cache clearing is necessary when a new submission is made to ensure the submissions list in the manage submissions page stays up-to-date. This implementation properly invalidates the cache at the right moment in the submission process.
app/controllers/submissions_controller.rb (2)
20-25: Efficient caching implementation for submission IDs.Caching submission IDs instead of entire objects is a good pattern for performance optimization. This reduces memory usage while still enabling quick data retrieval when needed.
27-35: Well-structured cache for submissions mapping.Good use of JSON serialization for the mapping data with appropriate expiration time. This will significantly reduce database load on the submissions page.
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.
Overall functionality seems to work the same, but does the caching work? I'm not sure if I ran something wrong, but the speed of loading the manage submissions page does not seem like it always goes down. At most on my machine, it goes down by ~10%, but sometimes it also increases after things are cached.
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.
After further discussion offline, this PR caches some parts of the manage submissions page, but a future pr will have to deal with caching the views, which is the main factor influencing loading times
* Caching some data from index, todo fix caching issues for submission data * Cached submission_ids instead of trying to do full data entries. * Changed expiration time of cache entries and allow for cache remake after new submission. * Uncommented necessary lines removed when testing. * Finished adding functionality of caching manage submissions. * Cleaned up code. (cherry picked from commit 9cc857f)
Description
cached entries in manage submissions to reduce load times
Motivation and Context
manage submissions loading time slow, hopefully reduced with caching
How Has This Been Tested?
Tested with creating new submissions (on manage submissions page), editing submissions, deleting submissions, and making new submissions. Also ensured current features on manage submissions page still work. Might've missed some functions, would greatly appreciate if checked to make sure caching does not break functionality.
Since it edits handin.rb, make sure to run rails server to make sure changes integrate.
Types of changes
Checklist:
overcommit --install && overcommit --signto use pre-commit hook for linting