edx-platform
edx-platform copied to clipboard
openedx
feat: [FC-0056] Implement Sidebar Navigation
Settings
COURSEWARE_MICROFRONTEND_NAVIGATION_SIDEBAR_BLOCKS_DISABLE_CACHING:
- name: courseware. disable_navigation_sidebar_blocks_caching
everyone: true
Description
This PR addresses to the need to add an API for Sidebar Navigation that returns the course structure with sections, subsections, and units, according to user rights.
To improve the performance of the API, was added caching of the course structure for the user, which makes it much easier to calculate the block structure for the user at each request. However, there may be cases when this caching can lead to an overflow of the cache storage in high-loaded LMS with large courses, so the corresponding flag "courseware. disable_navigation_sidebar_blocks_caching" was added so that this caching can be disabled.
Testing instructions
- Run master devstack.
- Start platform make dev.up and make checkout on this branch.
- Create a course with different access rights to course blocks (sections/subsections/subsections) for different users.
- Make GET requests to the API (/api/course_home/sidebar/{course_id}) from different users and check if the API returns the correct blocks depending on the user's permissions.
- Update the course and check if there are any new elements in the API response.
Thanks for the pull request, @NiedielnitsevIvan! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
- supporting documentation
- Open edX discussion forum threads
- timeline information ("this must be merged by XX date", and why that is)
- partner information ("this is a course on edx.org")
- any other information that can help Product understand the context for the PR
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
Please let us know once your PR is ready for our review and all tests are green.
@ormsbee Please check this the performance analysis on tutor dev - https://raccoongang.atlassian.net/wiki/external/MDhhMjdlMDhjODVkNDBjYjkzZjAzMDdhOGZmMGM2MTk
@GlugovGrGlib: Thank you! Do you have a broad sense of what the bottlenecks are for those really large courses?
@GlugovGrGlib: Just as a heads up, when I run this locally on my laptop with the large test course and try loading just this REST endpoint in isolation, I get speeds of about 4.7 seconds for staff and around 6 seconds for a student. That's about 3X faster than when I try to hit those endpoints on the sandbox (I was loading the REST endpoint directly there as well, instead of using the MFE, since I didn't want it to get slowed down by other requests). And that difference is with the debug toolbar left on. Is there a possibility that course blocks caching isn't properly configured on the sandbox?
@ormsbee Hello!
During testing, we found a bug that after changing the name in the course blocks (sections, subsections, and units), their name does not immediately change in the sidebar. A similar problem with the names of sections and subsections is currently present on the Course Outline page. The problem lies in the caching of the course structure at the BlockStore level.
One of the solutions to make the cache update instantly or much more often is to change the BlockStructureConfiguration, where you can specify the cache lifetime, but this can affect performance globally.
@NiedielnitsevIvan: Is that because the collect phase of block transformers takes time to run, so the version you get immediately after making a change is still the course blocks of the old course, and then that gets cached by this new code?
@NiedielnitsevIvan: Is that because the collect phase of block transformers takes time to run, so the version you get immediately after making a change is still the course blocks of the old course, and then that gets cached by this new code?
That's right, because the course version changes, and the blocks returned from get_course_outline_block_tree are still old, so cache invalidating by course version doesn't work in this case.
Do you have a broad sense of what the bottlenecks are for those really large courses?
Sorry, I really didn't go into the depth of the specific code that is the slowest to execute for some time already. Last time we needed to troubleshoot these issues for a client, it was around 2020-2021, when there weren't Learning MFE, and in parallel with your inputs to this discourse post, we got similar results at the time.
Additionally to this, recently we have tested fetching and processing the course structure to find the optimal course setup for a client, you might find those results insightful https://raccoongang.atlassian.net/wiki/external/NWJlYzYxYzRlYzRlNGE5YmFkYjkxYmE0ZTdlNTZjOWE.
Is there a possibility that course blocks caching isn't properly configured on the sandbox?
I believe you were right, initially I was confused about waffle flag, as It should be used to turn off caching, but was implemented the other way. Latter Ivan inverted the behavior for waffle flag, and the performance for loading from cache on the sandbox was enhanced.
@NiedielnitsevIvan 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.
2U Release Notice: This PR has been deployed to the edX production environment.
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.
2U Release Notice: This PR has been deployed to the edX production environment.
This PR is the part of the following product feature - https://github.com/openedx/platform-roadmap/issues/329