Skip to content

fix: recover from orphaned Mongo course index on course creation - #39024

Open
AhtishamShahid wants to merge 2 commits into
masterfrom
ahtishamshahid/fix-orphaned-mongo-course-index
Open

fix: recover from orphaned Mongo course index on course creation#39024
AhtishamShahid wants to merge 2 commits into
masterfrom
ahtishamshahid/fix-orphaned-mongo-course-index

Conversation

@AhtishamShahid

@AhtishamShahid AhtishamShahid commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Problem

The split modulestore course index is read from MySQL but written to both MySQL and Mongo (active_versions). Mongo writes aren't covered by ATOMIC_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:

  1. has_course() reads MySQL → course looks absent → creation is retried
  2. The follower write to Mongo hits UNIQUE(org, course, run)DuplicateKeyError
  3. Rollback again. Every retry fails identically, and nothing is left in MySQL to explain why

We 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, catch DuplicateKeyError from 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.

  • Without the fix: fails with DuplicateKeyError E11000 ... index: org_1_course_1_run_1
  • With the fix: TestCourseCreation and TestLibraries::test_create_library both pass

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.
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 25, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @AhtishamShahid!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform-oncall.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To 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:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where 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:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

4 participants