fix: recover from orphaned Mongo course index on course creation - #39024
fix: recover from orphaned Mongo course index on course creation#39024AhtishamShahid wants to merge 2 commits into
Conversation
The split modulestore course index is read from MySQL but written to both MySQL and Mongo's active_versions. Mongo writes are not covered by ATOMIC_REQUESTS, so if a request creates a course and then fails, the MySQL row is rolled back while the Mongo doc survives. That course key is then permanently unusable: has_course() reads MySQL and reports the course as absent, so creation is retried, and the follower write to Mongo raises DuplicateKeyError against UNIQUE(org, course, run). Every retry fails identically, and the rollback leaves no MySQL trace of why. Clear any stale Mongo doc before the follower write. This is safe because it runs after new_index.save() has succeeded, which means no MySQL row existed for the key, which means any Mongo doc for it is stale.
|
Thanks for the pull request, @AhtishamShahid! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Deleting unconditionally added a Mongo round-trip to every course and library creation, which broke check_mongo_calls in TestLibraries::test_create_library (expected 3 calls, 4 were made). Insert first and clean up only on DuplicateKeyError, so the happy path keeps its original call count and the extra work happens only in the rare case where a stale doc is actually present.
There was a problem hiding this comment.
Pull request overview
Recovers course creation when an orphaned MongoDB course index conflicts with the authoritative MySQL index.
Changes:
- Removes stale MongoDB index documents on duplicate-key conflicts and retries once.
- Adds regression coverage reproducing the orphaned-index state.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
xmodule/modulestore/split_mongo/mongo_connection.py |
Implements conflict recovery during index insertion. |
xmodule/modulestore/tests/test_split_modulestore.py |
Tests successful recreation and index consistency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Problem
The split modulestore course index is read from MySQL but written to both MySQL and Mongo (
active_versions). Mongo writes aren't covered byATOMIC_REQUESTS, so if a request creates a course and then fails later, MySQL rolls back but the Mongo doc survives.That course key then becomes permanently unusable:
has_course()reads MySQL → course looks absent → creation is retriedUNIQUE(org, course, run)→DuplicateKeyErrorWe hit this on a production instance: a course import failed once, and after that the same course could never be imported again. Two course keys were affected, one wedged silently since 2022.
Fix
In
insert_course_index, catchDuplicateKeyErrorfrom the Mongo write, drop the stale doc, and retry once.This is safe because it runs after
new_index.save()succeeded — which means no MySQL row existed for the key, which means any Mongo doc for it is stale by definition. A warning is logged when one is removed.Cleaning up only on conflict (rather than unconditionally before the insert) keeps the happy path at its original number of Mongo round-trips, so normal course and library creation is unaffected.
Testing
Added
TestCourseCreation.test_creation_recovers_from_orphaned_mongo_index, which builds the exact state (create a course, then drop only the MySQL row) and asserts recreation succeeds without duplicating the doc.The test calls
store.ensure_indexes()so the unique index is present, matching a real deployment — without it Mongo silently accepts a second doc instead of raising, and the test wouldn't guard anything.DuplicateKeyError E11000 ... index: org_1_course_1_run_1TestCourseCreationandTestLibraries::test_create_libraryboth pass