Skip to content

Commit

Permalink
[SE-4101] fix: address VisibleBlocks caching race condition (openedx#…
Browse files Browse the repository at this point in the history
…27359) (#267)

* 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 <[email protected]>
(cherry picked from commit 6ccdaca)

Co-authored-by: Gábor Boros <[email protected]>
  • Loading branch information
asadmanzoor93 and gabor-boros authored Aug 30, 2021
1 parent 1c6eac2 commit 28b3f42
Showing 1 changed file with 34 additions and 5 deletions.
39 changes: 34 additions & 5 deletions lms/djangoapps/grades/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 28b3f42

Please sign in to comment.