feat: remove mongodb_proxy in favor of MongoDB's built-in retryReads (#37011)

This PR removes the usage of the custom `mongodb_proxy` package from
edx-platform, replacing it with MongoDB's built-in retry functionality via
`retryReads=True`.

Changes:

* Removed all references to `mongodb_proxy.MongoProxy` from connection logic.
* Updated `connect_to_mongodb()` to always use `retryReads=True`.
* Uninstalled `openedx-mongodbproxy` from edx-platform requirements.

Fixes: #35210

Author: wgu-ram-chandra

requirements/edx/base.txt

@@ -844,8 +844,6 @@ openedx-learning==0.26.0
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/kernel.in
-openedx-mongodbproxy==0.3.0
-    # via -r requirements/edx/kernel.in
 optimizely-sdk==5.2.0
     # via -r requirements/edx/bundled.in
 ora2==6.16.3
@@ -958,7 +956,6 @@ pymongo==4.4.0
     #   event-tracking
     #   mongoengine
     #   openedx-forum
-    #   openedx-mongodbproxy
 pynacl==1.5.0
     # via
     #   edx-django-utils

requirements/edx/development.txt

@@ -1405,10 +1405,6 @@ openedx-learning==0.26.0
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
-openedx-mongodbproxy==0.3.0
-    # via
-    #   -r requirements/edx/doc.txt
-    #   -r requirements/edx/testing.txt
 optimizely-sdk==5.2.0
     # via
     #   -r requirements/edx/doc.txt
@@ -1654,7 +1650,6 @@ pymongo==4.4.0
     #   event-tracking
     #   mongoengine
     #   openedx-forum
-    #   openedx-mongodbproxy
 pynacl==1.5.0
     # via
     #   -r requirements/edx/doc.txt

requirements/edx/doc.txt

@@ -1009,8 +1009,6 @@ openedx-learning==0.26.0
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
-openedx-mongodbproxy==0.3.0
-    # via -r requirements/edx/base.txt
 optimizely-sdk==5.2.0
     # via -r requirements/edx/base.txt
 ora2==6.16.3
@@ -1163,7 +1161,6 @@ pymongo==4.4.0
     #   event-tracking
     #   mongoengine
     #   openedx-forum
-    #   openedx-mongodbproxy
 pynacl==1.5.0
     # via
     #   -r requirements/edx/base.txt

requirements/edx/kernel.in

@@ -121,7 +121,6 @@ openedx-events                      # Open edX Events from Hooks Extension Frame
 openedx-filters                     # Open edX Filters from Hooks Extension Framework (OEP-50)
 openedx-forum                       # Open edX forum v2 application
 openedx-learning                    # Open edX Learning core (experimental)
-openedx-mongodbproxy
 openedx-django-wiki
 path
 piexif                              # Exif image metadata manipulation, used in the profile_images app

requirements/edx/testing.txt

@@ -1068,8 +1068,6 @@ openedx-learning==0.26.0
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt
-openedx-mongodbproxy==0.3.0
-    # via -r requirements/edx/base.txt
 optimizely-sdk==5.2.0
     # via -r requirements/edx/base.txt
 ora2==6.16.3
@@ -1255,7 +1253,6 @@ pymongo==4.4.0
     #   event-tracking
     #   mongoengine
     #   openedx-forum
-    #   openedx-mongodbproxy
 pynacl==1.5.0
     # via
     #   -r requirements/edx/base.txt

xmodule/contentstore/mongo.py

@@ -12,7 +12,6 @@
 from bson.son import SON
 from fs.osfs import OSFS
 from gridfs.errors import NoFile, FileExists
-from mongodb_proxy import autoretry_read
 from opaque_keys.edx.keys import AssetKey
 
 from xmodule.contentstore.content import XASSET_LOCATION_TAG
@@ -29,6 +28,7 @@ class MongoContentStore(ContentStore):
     MongoDB-backed ContentStore.
     """
     # lint-amnesty, pylint: disable=unused-argument
+
     def __init__(
         self, host, db,
         port=27017, tz_aware=True, user=None, password=None, bucket='fs', collection=None, **kwargs
@@ -39,16 +39,13 @@ def __init__(
         :param collection: ignores but provided for consistency w/ other doc_store_config patterns
         """
         # GridFS will throw an exception if the Database is wrapped in a MongoProxy. So don't wrap it.
-        # The appropriate methods below are marked as autoretry_read - those methods will handle
-        # the AutoReconnect errors.
         self.connection_params = {
             'db': db,
             'host': host,
             'port': port,
             'tz_aware': tz_aware,
             'user': user,
             'password': password,
-            'proxy': False,
             **kwargs
         }
         self.bucket = bucket
@@ -164,7 +161,6 @@ def delete(self, location_or_id):
         # Deletes of non-existent files are considered successful
         self.fs.delete(location_or_id)
 
-    @autoretry_read()
     def find(self, location, throw_on_not_found=True, as_stream=False):  # lint-amnesty, pylint: disable=arguments-differ
         content_id, __ = self.asset_db_key(location)
 
@@ -292,7 +288,6 @@ def remove_redundant_content_for_courses(self):
             self.fs_files.remove(query)
         return assets_to_delete
 
-    @autoretry_read()
     def _get_all_content_for_course(self,
                                     course_key,
                                     get_thumbnails=False,
@@ -404,7 +399,6 @@ def set_attrs(self, location, attr_dict):
         if result.matched_count == 0:
             raise NotFoundError(asset_db_key)
 
-    @autoretry_read()
     def get_attrs(self, location):
         """
         Gets all of the attributes associated with the given asset. Note, returns even built in attrs

xmodule/modulestore/mongo/base.py

@@ -23,7 +23,6 @@
 import pymongo
 from bson.son import SON
 from fs.osfs import OSFS
-from mongodb_proxy import autoretry_read
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryLocator
 from path import Path as path
@@ -85,6 +84,7 @@ class MongoKeyValueStore(InheritanceKeyValueStore):
     A KeyValueStore that maps keyed data access to one of the 3 data areas
     known to the MongoModuleStore (data, children, and metadata)
     """
+
     def __init__(self, data, metadata):
         super().__init__()
         if not isinstance(data, dict):
@@ -465,7 +465,6 @@ def __init__(self, contentstore, doc_store_config, fs_root, render_template,
                  fs_service=None,
                  user_service=None,
                  signal_handler=None,
-                 retry_wait_time=0.1,
                  **kwargs):
         """
         :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware.
@@ -474,7 +473,6 @@ def __init__(self, contentstore, doc_store_config, fs_root, render_template,
         super().__init__(contentstore=contentstore, **kwargs)
 
         self.doc_store_config = doc_store_config
-        self.retry_wait_time = retry_wait_time
         self.do_connection(**self.doc_store_config)
 
         if default_class is not None:
@@ -534,7 +532,7 @@ def do_connection(
         self.database = connect_to_mongodb(
             db, host,
             port=port, tz_aware=tz_aware, user=user, password=password,
-            retry_wait_time=self.retry_wait_time, **kwargs
+            **kwargs
         )
 
         self.collection = self.database[collection]
@@ -569,7 +567,7 @@ def _drop_database(self, database=True, collections=True, connections=True):
         connection = self.collection.database.client
 
         if database:
-            connection.drop_database(self.collection.database.proxied_object)
+            connection.drop_database(self.collection.database)
         elif collections:
             self.collection.drop()
         else:
@@ -578,7 +576,6 @@ def _drop_database(self, database=True, collections=True, connections=True):
         if connections:
             connection.close()
 
-    @autoretry_read()
     def fill_in_run(self, course_key):
         """
         In mongo some course_keys are used without runs. This helper function returns
@@ -737,7 +734,6 @@ def _load_items(self, course_key, items, using_descriptor_system=None, for_paren
             for item in items
         ]
 
-    @autoretry_read()
     def get_course_summaries(self, **kwargs):
         """
         Returns a list of `CourseSummary`. This accepts an optional parameter of 'org' which
@@ -786,7 +782,6 @@ def extract_course_summary(course):
 
         return courses_summaries
 
-    @autoretry_read()
     def get_courses(self, **kwargs):
         '''
         Returns a list of course descriptors. This accepts an optional parameter of 'org' which
@@ -821,7 +816,6 @@ def get_courses(self, **kwargs):
         )
         return [course for course in base_list if not isinstance(course, ErrorBlock)]
 
-    @autoretry_read()
     def _find_one(self, location):
         '''Look for a given location in the collection. If the item is not present, raise
         ItemNotFoundError.
@@ -867,7 +861,6 @@ def get_course(self, course_key, **kwargs):  # lint-amnesty, pylint: disable=arg
         except ItemNotFoundError:
             return None
 
-    @autoretry_read()
     def has_course(self, course_key, ignore_case=False, **kwargs):  # lint-amnesty, pylint: disable=arguments-differ
         """
         Returns the course_id of the course if it was found, else None
@@ -962,7 +955,6 @@ def _id_dict_to_son(id_dict):
             for key in ('tag', 'org', 'course', 'category', 'name', 'revision')
         ])
 
-    @autoretry_read()
     def get_items(  # lint-amnesty, pylint: disable=arguments-differ
             self,
             course_id,

xmodule/modulestore/split_mongo/mongo_connection.py

@@ -17,7 +17,6 @@
 from django.db.transaction import TransactionManagementError
 import pymongo
 import pytz
-from mongodb_proxy import autoretry_read
 # Import this just to export it
 from pymongo.errors import DuplicateKeyError  # pylint: disable=unused-import
 from edx_django_utils import monitoring
@@ -57,6 +56,7 @@ class Tagger:
     An object used by :class:`QueryTimer` to allow timed code blocks
     to add measurements and tags to the timer.
     """
+
     def __init__(self, default_sample_rate):
         self.added_tags = []
         self.measures = []
@@ -103,6 +103,7 @@ class QueryTimer:
     An object that allows timing a block of code while also recording measurements
     about that code.
     """
+
     def __init__(self, metric_base, sample_rate=1):
         """
         Arguments:
@@ -198,6 +199,7 @@ class CourseStructureCache:
     If the 'course_structure_cache' doesn't exist, then don't do anything for
     for set and get.
     """
+
     def __init__(self):
         self.cache = None
         try:
@@ -264,9 +266,10 @@ class MongoPersistenceBackend:
     """
     Segregation of pymongo functions from the data modeling mechanisms for split modulestore.
     """
+
     def __init__(
         self, db, collection, host, port=27017, tz_aware=True, user=None, password=None,
-        asset_collection=None, retry_wait_time=0.1, with_mysql_subclass=False, **kwargs  # lint-amnesty, pylint: disable=unused-argument
+        asset_collection=None, with_mysql_subclass=False, **kwargs  # lint-amnesty, pylint: disable=unused-argument
     ):
         """
         Create & open the connection, authenticate, and provide pointers to the collections
@@ -286,7 +289,6 @@ def __init__(
             'tz_aware': tz_aware,
             'user': user,
             'password': password,
-            'retry_wait_time': retry_wait_time,
             **kwargs
         }
 
@@ -362,7 +364,6 @@ def get_structure(self, key, course_context=None):
 
             return structure
 
-    @autoretry_read()
     def find_structures_by_id(self, ids, course_context=None):
         """
         Return all structures that specified in ``ids``.
@@ -379,7 +380,6 @@ def find_structures_by_id(self, ids, course_context=None):
             tagger.measure("structures", len(docs))
             return docs
 
-    @autoretry_read()
     def find_courselike_blocks_by_id(self, ids, block_type, course_context=None):
         """
         Find all structures that specified in `ids`. Among the blocks only return block whose type is `block_type`.

xmodule/modulestore/split_mongo/split.py

@@ -63,7 +63,6 @@
 
 from bson.objectid import ObjectId
 from ccx_keys.locator import CCXBlockUsageLocator, CCXLocator
-from mongodb_proxy import autoretry_read
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import (
     BlockUsageLocator,
@@ -953,7 +952,6 @@ def _create_library_locator(self, library_info, branch):
             branch=branch,
         )
 
-    @autoretry_read()
     def get_courses(self, branch, **kwargs):  # lint-amnesty, pylint: disable=arguments-differ
         """
         Returns a list of course blocks matching any given qualifiers.
@@ -969,7 +967,6 @@ def get_courses(self, branch, **kwargs):  # lint-amnesty, pylint: disable=argume
         # get the blocks for each course index (s/b the root)
         return self._get_structures_for_branch_and_locator(branch, self._create_course_locator, **kwargs)
 
-    @autoretry_read()
     def get_course_summaries(self, branch, **kwargs):
         """
         Returns a list of `CourseSummary` which matching any given qualifiers.
@@ -1026,7 +1023,6 @@ def get_library_keys(self):
             in self.find_matching_course_indexes(branch="library")
         })
 
-    @autoretry_read()
     def get_library_summaries(self, **kwargs):
         """
         Returns a list of `LegacyLibrarySummary` objects.
@@ -3255,7 +3251,6 @@ def _update_block_in_structure(self, structure, block_key, content):
         """
         structure['blocks'][block_key] = content
 
-    @autoretry_read()
     def find_courses_by_search_target(self, field_name, field_value):
         """
         Find all the courses which cached that they have the given field with the given value.
@@ -3325,6 +3320,7 @@ class SparseList(list):
     Enable inserting items into a list in arbitrary order and then retrieving them.
     """
     # taken from http://stackoverflow.com/questions/1857780/sparse-assignment-list-in-python
+
     def __setitem__(self, index, value):
         """
         Add value to the list ensuring the list is long enough to accommodate it at the given index

xmodule/modulestore/tests/test_split_mongo_mongo_connection.py

@@ -14,13 +14,12 @@
 class TestHeartbeatFailureException(unittest.TestCase):
     """ Test that a heartbeat failure is thrown at the appropriate times """
 
-    @patch('pymongo.MongoClient')
-    @patch('pymongo.database.Database')
-    def test_heartbeat_raises_exception_when_connection_alive_is_false(self, *calls):
+    @patch('pymongo.mongo_client.MongoClient')
+    def test_heartbeat_raises_exception_when_connection_alive_is_false(self, MockClient):
         # pylint: disable=W0613
-        with patch('mongodb_proxy.MongoProxy') as mock_proxy:
-            mock_proxy.return_value.admin.command.side_effect = ConnectionFailure('Test')
-            useless_conn = MongoPersistenceBackend('useless', 'useless', 'useless')
 
-            with pytest.raises(HeartbeatFailure):
-                useless_conn.heartbeat()
+        MockClient.return_value.admin.command.side_effect = ConnectionFailure('Test')
+        useless_conn = MongoPersistenceBackend('useless', 'useless', 'useless')
+
+        with pytest.raises(HeartbeatFailure):
+            useless_conn.heartbeat()