From 3d2995e485837ac6ac59564f97de7690060120b0 Mon Sep 17 00:00:00 2001 From: Ahtisahm Shahid Date: Tue, 25 Aug 2026 17:19:19 +0500 Subject: [PATCH 1/2] fix: recover from orphaned Mongo course index on course creation 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. --- .../split_mongo/mongo_connection.py | 20 ++++++++ .../tests/test_split_modulestore.py | 47 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/xmodule/modulestore/split_mongo/mongo_connection.py b/xmodule/modulestore/split_mongo/mongo_connection.py index a818c04ede62..79e8463ea39c 100644 --- a/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/xmodule/modulestore/split_mongo/mongo_connection.py @@ -741,6 +741,26 @@ def insert_course_index(self, course_index, course_context=None): # pylint: dis # Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well. # NOTE: This is REQUIRED for pruning (structures.py) to run safely. Don't remove this write until # pruning is modified to read from SplitModulestoreCourseIndex to get active versions. + # + # First clear any stale MongoDB doc for this course. MySQL is the source of truth for which courses + # exist, and new_index.save() above has already succeeded, so we know no MySQL row existed for this + # key; any MongoDB doc for it is therefore stale. Such a doc is left behind whenever an earlier + # request got this far and then had its MySQL transaction rolled back, because MongoDB writes are + # not covered by ATOMIC_REQUESTS. Without this cleanup, the insert below raises DuplicateKeyError + # against the UNIQUE(org, course, run) index on active_versions and the course can never be created + # again -- every retry fails identically, and the rollback leaves no MySQL trace to explain why. + stale_docs = self.course_index.delete_many({ + 'org': course_index['org'], + 'course': course_index['course'], + 'run': course_index['run'], + }).deleted_count + if stale_docs: + log.warning( + "Removed %d stale MongoDB active_versions doc(s) for %s/%s/%s which had no " + "SplitModulestoreCourseIndex row. This usually means an earlier attempt to create this " + "course wrote to MongoDB and then had its MySQL transaction rolled back.", + stale_docs, course_index['org'], course_index['course'], course_index['run'], + ) super().insert_course_index(course_index, course_context) def update_course_index(self, course_index, from_index=None, course_context=None): # pylint: disable=arguments-differ diff --git a/xmodule/modulestore/tests/test_split_modulestore.py b/xmodule/modulestore/tests/test_split_modulestore.py index 1d1c54859b4e..18131459a912 100644 --- a/xmodule/modulestore/tests/test_split_modulestore.py +++ b/xmodule/modulestore/tests/test_split_modulestore.py @@ -18,6 +18,7 @@ from opaque_keys.edx.locator import BlockUsageLocator, CourseKey, CourseLocator, LocalId from xblock.fields import Date, Reference, ReferenceList, ReferenceValueDict, Timedelta +from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex from openedx.core.djangolib.testing.utils import CacheIsolationMixin from openedx.core.lib import tempdir from openedx.core.lib.tests import attr @@ -1643,6 +1644,52 @@ def test_simple_creation(self): assert len(new_course.grading_policy['GRADER']) == 4 self.assertDictEqual(new_course.grade_cutoffs, {"Pass": 0.5}) # noqa: PT009 + def test_creation_recovers_from_orphaned_mongo_index(self): + """ + Creating a course must succeed even if MongoDB's active_versions still holds a doc for that + org/course/run while MySQL has no SplitModulestoreCourseIndex row. + + This state arises whenever an earlier attempt to create the course wrote the MongoDB doc and then + had its MySQL transaction rolled back -- MongoDB writes are not covered by ATOMIC_REQUESTS. MySQL is + the source of truth for which courses exist, so `has_course()` reports the course as absent and + creation is attempted again. Before this was fixed, the follower write to MongoDB used insert_one() + and raised DuplicateKeyError against the UNIQUE(org, course, run) index on active_versions, so the + course could never be created again -- every retry failed identically, and the rollback left no + MySQL trace to explain why. + """ + store = modulestore() + # Provision the UNIQUE(org, course, run) index on active_versions that deployments get from the + # `ensure_indexes` management command. Without it MongoDB silently accepts a second doc for the + # same course instead of raising, which is not the behaviour we need to guard against here. + store.ensure_indexes() + org, course, run = 'orphan_org', 'orphan_course', 'orphan_run' + created = store.create_course(org, course, run, TEST_USER_ID, BRANCH_NAME_DRAFT) + course_key = created.location.course_key + + # Simulate the rolled-back request: drop only the MySQL row, leaving the MongoDB doc orphaned. + # Deliberately bypasses delete_course_index(), which would clear both stores. + SplitModulestoreCourseIndex.objects.filter( + course_id=course_key.for_branch(None).version_agnostic() + ).delete() + db_connection = store.db_connection + mongo_query = {'org': org, 'course': course, 'run': run} + assert db_connection.course_index.count_documents(mongo_query) == 1, "expected an orphaned Mongo doc" + assert store.has_course(CourseLocator(org, course, run)) is None, "MySQL should report absence" + + # Creating the course again must now succeed rather than raising DuplicateKeyError. + recreated = store.create_course(org, course, run, TEST_USER_ID, BRANCH_NAME_DRAFT) + recreated_key = recreated.location.course_key + + # The stale doc is replaced, not duplicated, and the two stores agree again. + assert db_connection.course_index.count_documents(mongo_query) == 1 + mongo_doc = db_connection.course_index.find_one(mongo_query) + mysql_row = SplitModulestoreCourseIndex.objects.get( + course_id=recreated_key.for_branch(None).version_agnostic() + ) + assert str(mongo_doc['_id']) == mysql_row.objectid + assert mongo_doc['versions'][BRANCH_NAME_DRAFT] == \ + store.get_course_index_info(recreated_key)['versions'][BRANCH_NAME_DRAFT] + def test_cloned_course(self): """ Test making a course which points to an existing draft and published but not making any changes to either. From 259651253b01d5efd25085d00bdfdcd140965d1e Mon Sep 17 00:00:00 2001 From: Ahtisahm Shahid Date: Tue, 25 Aug 2026 17:43:23 +0500 Subject: [PATCH 2/2] fix: only clean up the stale Mongo doc when the insert conflicts 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. --- .../split_mongo/mongo_connection.py | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/xmodule/modulestore/split_mongo/mongo_connection.py b/xmodule/modulestore/split_mongo/mongo_connection.py index 79e8463ea39c..e52de5488231 100644 --- a/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/xmodule/modulestore/split_mongo/mongo_connection.py @@ -24,7 +24,7 @@ from edx_django_utils.cache import RequestCache # Import this just to export it -from pymongo.errors import DuplicateKeyError # pylint: disable=unused-import # noqa: F401 +from pymongo.errors import DuplicateKeyError from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex from openedx.core.lib.cache_utils import request_cached @@ -741,27 +741,28 @@ def insert_course_index(self, course_index, course_context=None): # pylint: dis # Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well. # NOTE: This is REQUIRED for pruning (structures.py) to run safely. Don't remove this write until # pruning is modified to read from SplitModulestoreCourseIndex to get active versions. - # - # First clear any stale MongoDB doc for this course. MySQL is the source of truth for which courses - # exist, and new_index.save() above has already succeeded, so we know no MySQL row existed for this - # key; any MongoDB doc for it is therefore stale. Such a doc is left behind whenever an earlier - # request got this far and then had its MySQL transaction rolled back, because MongoDB writes are - # not covered by ATOMIC_REQUESTS. Without this cleanup, the insert below raises DuplicateKeyError - # against the UNIQUE(org, course, run) index on active_versions and the course can never be created - # again -- every retry fails identically, and the rollback leaves no MySQL trace to explain why. - stale_docs = self.course_index.delete_many({ - 'org': course_index['org'], - 'course': course_index['course'], - 'run': course_index['run'], - }).deleted_count - if stale_docs: + try: + super().insert_course_index(course_index, course_context) + except DuplicateKeyError: + # A MongoDB doc already exists for this org/course/run, but MySQL is the source of truth for + # which courses exist and new_index.save() above has already succeeded, so we know no MySQL row + # existed for this key. The MongoDB doc is therefore stale: it is left behind whenever an + # earlier request got this far and then had its MySQL transaction rolled back, since MongoDB + # writes are not covered by ATOMIC_REQUESTS. Drop it and retry, otherwise this course could + # never be created again -- every attempt would fail here identically, and the rollback leaves + # no MySQL trace to explain why. + stale_docs = self.course_index.delete_many({ + 'org': course_index['org'], + 'course': course_index['course'], + 'run': course_index['run'], + }).deleted_count log.warning( "Removed %d stale MongoDB active_versions doc(s) for %s/%s/%s which had no " "SplitModulestoreCourseIndex row. This usually means an earlier attempt to create this " "course wrote to MongoDB and then had its MySQL transaction rolled back.", stale_docs, course_index['org'], course_index['course'], course_index['run'], ) - super().insert_course_index(course_index, course_context) + super().insert_course_index(course_index, course_context) def update_course_index(self, course_index, from_index=None, course_context=None): # pylint: disable=arguments-differ """