From 28b3f429f69eb3e4de0ff01e926f6348479360c4 Mon Sep 17 00:00:00 2001 From: Asad Manzoor Date: Mon, 30 Aug 2021 14:24:23 +0500 Subject: [PATCH] [SE-4101] fix: address VisibleBlocks caching race condition (#27359) (#267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: address VisibleBlocks caching race condition * sets visual block creation in an atomic transaction * refactor: add logging statement to bulk create Co-authored-by: Raul Gallegos (cherry picked from commit 6ccdaca3455bb78ca71065a7fca0f758fe90f248) Co-authored-by: Gábor Boros --- lms/djangoapps/grades/models.py | 39 ++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 3a461d5e4d8d..fe0026b463a3 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -18,7 +18,7 @@ import six from django.apps import apps from django.contrib.auth.models import User -from django.db import models +from django.db import models, IntegrityError, transaction from django.utils.encoding import python_2_unicode_compatible from django.utils.timezone import now from lazy import lazy @@ -212,16 +212,45 @@ def bulk_create(cls, user_id, course_key, block_record_lists): for the block records' course with the new VisibleBlocks. Returns the newly created visible blocks. """ - created = cls.objects.bulk_create([ + visual_blocks = [ VisibleBlocks( blocks_json=brl.json_value, hashed=brl.hash_value, course_id=course_key, ) for brl in block_record_lists - ]) - cls._update_cache(user_id, course_key, created) - return created + ] + + created_visual_blocks = [] + existing_visual_blocks = [] + + try: + # Try to bulk create the blocks assuming all blocks are new + with transaction.atomic(): + created_visual_blocks = cls.objects.bulk_create(visual_blocks) + except IntegrityError: + log.warning('Falling back to create VisualBlocks one by one for user {} in course {}'.format( + user_id, + course_key + )) + + # Try to create blocks one by one and mark newly created blocks + for visual_block in visual_blocks: + existing_blocks = cls.objects.filter(hashed=visual_block.hashed) + + if existing_blocks.exists(): + # As only one record has a matching hash it is safe to use first + existing_visual_blocks.append(existing_blocks.first()) + else: + # Create the visual block and add mark as newly created + visual_block.save() + created_visual_blocks.append(visual_block) + + # Update the cache with the conjunction of created and existing blocks + cls._update_cache(user_id, course_key, existing_visual_blocks + created_visual_blocks) + + # Return the new visual blocks + return created_visual_blocks @classmethod def bulk_get_or_create(cls, user_id, course_key, block_record_lists):