diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index d06db8fd6f..faf2bfa01d 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -229,18 +229,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (20, 6), - ('no_overrides', 2, True, False): (20, 6), - ('no_overrides', 3, True, False): (20, 6), - ('ccx', 1, True, False): (20, 6), - ('ccx', 2, True, False): (20, 6), - ('ccx', 3, True, False): (20, 6), - ('no_overrides', 1, False, False): (20, 6), - ('no_overrides', 2, False, False): (20, 6), - ('no_overrides', 3, False, False): (20, 6), - ('ccx', 1, False, False): (20, 6), - ('ccx', 2, False, False): (20, 6), - ('ccx', 3, False, False): (20, 6), + ('no_overrides', 1, True, False): (22, 6), + ('no_overrides', 2, True, False): (22, 6), + ('no_overrides', 3, True, False): (22, 6), + ('ccx', 1, True, False): (22, 6), + ('ccx', 2, True, False): (22, 6), + ('ccx', 3, True, False): (22, 6), + ('no_overrides', 1, False, False): (22, 6), + ('no_overrides', 2, False, False): (22, 6), + ('no_overrides', 3, False, False): (22, 6), + ('ccx', 1, False, False): (22, 6), + ('ccx', 2, False, False): (22, 6), + ('ccx', 3, False, False): (22, 6), } @@ -252,19 +252,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (20, 3), - ('no_overrides', 2, True, False): (20, 3), - ('no_overrides', 3, True, False): (20, 3), - ('ccx', 1, True, False): (20, 3), - ('ccx', 2, True, False): (20, 3), - ('ccx', 3, True, False): (20, 3), - ('ccx', 1, True, True): (21, 3), - ('ccx', 2, True, True): (21, 3), - ('ccx', 3, True, True): (21, 3), - ('no_overrides', 1, False, False): (20, 3), - ('no_overrides', 2, False, False): (20, 3), - ('no_overrides', 3, False, False): (20, 3), - ('ccx', 1, False, False): (20, 3), - ('ccx', 2, False, False): (20, 3), - ('ccx', 3, False, False): (20, 3), + ('no_overrides', 1, True, False): (22, 3), + ('no_overrides', 2, True, False): (22, 3), + ('no_overrides', 3, True, False): (22, 3), + ('ccx', 1, True, False): (22, 3), + ('ccx', 2, True, False): (22, 3), + ('ccx', 3, True, False): (22, 3), + ('ccx', 1, True, True): (23, 3), + ('ccx', 2, True, True): (23, 3), + ('ccx', 3, True, True): (23, 3), + ('no_overrides', 1, False, False): (22, 3), + ('no_overrides', 2, False, False): (22, 3), + ('no_overrides', 3, False, False): (22, 3), + ('ccx', 1, False, False): (22, 3), + ('ccx', 2, False, False): (22, 3), + ('ccx', 3, False, False): (22, 3), } diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index a73ae740c0..08e1d0ce91 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1345,17 +1345,17 @@ class ProgressPageTests(ModuleStoreTestCase): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(37), check_mongo_calls(4): + with self.assertNumQueries(39), check_mongo_calls(4): self._get_progress_page() def test_progress_queries(self): self.setup_course() - with self.assertNumQueries(37), check_mongo_calls(4): + with self.assertNumQueries(39), check_mongo_calls(4): self._get_progress_page() # subsequent accesses to the progress page require fewer queries. for _ in range(2): - with self.assertNumQueries(20), check_mongo_calls(4): + with self.assertNumQueries(22), check_mongo_calls(4): self._get_progress_page() @patch( diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index 5b8ea8761a..b9c6e140aa 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/lms/djangoapps/grades/new/subsection_grade.py @@ -2,6 +2,9 @@ SubsectionGrade Class """ from collections import OrderedDict +from contextlib import contextmanager +from django.db import transaction +from django.db.utils import DatabaseError from lazy import lazy from logging import getLogger from courseware.model_data import ScoresClient @@ -10,6 +13,7 @@ from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from student.models import anonymous_id_for_user, User from submissions import api as submissions_api +from traceback import format_exc from xmodule import block_metadata_utils, graders from xmodule.graders import Score @@ -17,6 +21,20 @@ from xmodule.graders import Score log = getLogger(__name__) +@contextmanager +def persistence_safe_fallback(): + """ + A context manager intended to be used to wrap robust grades database-specific code that can be ignored on failure. + """ + try: + with transaction.atomic(): + yield + except DatabaseError: + # Error deliberately logged, then swallowed. It's assumed that the wrapped code is not guaranteed to succeed. + # TODO: enqueue a celery task to finish the task asynchronously, see TNL-5471 + log.warning("Persistent Grades: Persistence Error, falling back.\n{}".format(format_exc())) + + class SubsectionGrade(object): """ Class for Subsection Grades. @@ -254,8 +272,9 @@ class SubsectionGradeFactory(object): if read_only: self._unsaved_subsection_grades.append(subsection_grade) else: - grade_model = subsection_grade.create_model(self.student) - self._update_saved_subsection_grade(subsection.location, grade_model) + with persistence_safe_fallback(): + grade_model = subsection_grade.create_model(self.student) + self._update_saved_subsection_grade(subsection.location, grade_model) return subsection_grade def bulk_create_unsaved(self): @@ -264,8 +283,9 @@ class SubsectionGradeFactory(object): """ self._log_event(log.warning, u"bulk_create_unsaved") - SubsectionGrade.bulk_create_models(self.student, self._unsaved_subsection_grades, self.course.id) - self._unsaved_subsection_grades = [] + with persistence_safe_fallback(): + SubsectionGrade.bulk_create_models(self.student, self._unsaved_subsection_grades, self.course.id) + self._unsaved_subsection_grades = [] def update(self, subsection, block_structure=None): """ diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index 777f4f37dc..eaa8fe7bda 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/lms/djangoapps/grades/tests/test_new.py @@ -4,6 +4,7 @@ Test saved subsection grade functionality. import ddt from django.conf import settings +from django.db.utils import DatabaseError from mock import patch from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory @@ -117,7 +118,7 @@ class SubsectionGradeFactoryTest(GradeTestBase): 'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_saved_grade', wraps=self.subsection_grade_factory._get_saved_grade # pylint: disable=protected-access ) as mock_get_saved_grade: - with self.assertNumQueries(12): + with self.assertNumQueries(14): grade_a = self.subsection_grade_factory.create(self.sequence) self.assertTrue(mock_get_saved_grade.called) self.assertTrue(mock_create_grade.called) @@ -133,6 +134,30 @@ class SubsectionGradeFactoryTest(GradeTestBase): self.assertEqual(grade_a.url_name, grade_b.url_name) self.assertEqual(grade_a.all_total, grade_b.all_total) + @ddt.data( + ( + 'lms.djangoapps.grades.new.subsection_grade.SubsectionGrade.create_model', + lambda self: self.subsection_grade_factory.create(self.sequence) + ), + ( + 'lms.djangoapps.grades.new.subsection_grade.SubsectionGrade.bulk_create_models', + lambda self: self.subsection_grade_factory.bulk_create_unsaved() + ), + ) + @ddt.unpack + def test_fallback_handling(self, underlying_method, method_to_test): + """ + Tests that the persistent grades fallback handler functions as expected. + """ + with patch('lms.djangoapps.grades.new.subsection_grade.log') as log_mock: + with patch(underlying_method) as underlying: + underlying.side_effect = DatabaseError("I'm afraid I can't do that") + method_to_test(self) + # By making it this far, we implicitly assert "the factory method swallowed the exception correctly" + self.assertTrue( + log_mock.warning.call_args_list[0].startswith("Persistent Grades: Persistence Error, falling back.") + ) + @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.data( (True, True), diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 21f693d246..94be388f4b 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1670,7 +1670,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 3, 'skipped': 2 } - with self.assertNumQueries(175): + with self.assertNumQueries(191): self.assertCertificatesGenerated(task_input, expected_results) expected_results = {