[BD-14] fix: call blockstore APIs in atomic transactions.

[pomegranited]

Description

Ensures data integrity of content library views when accessing blockstore as an app, running on a separate database from edxapp.

Background

edx-platform#29779 moved the blockstore app into edx-platform. On edx.org, blockstore was being run as a separate IDA, and therefore used its own database. Now that they're being run together under the same service umbrella, the blockstore database is configured as a secondary database in Django settings, and there's a database router which tells the service which database to use for which model requests.

When blockstore runs on the IDA, it has ATOMIC_REQUESTS: true enabled so that all views are encased in a single transaction. This works great when you only have a single database running for a service, and you want all the views for that service to be atomic. But when you have multiple databases, if any database has ATOMIC_REQUESTS:true, then all views are made atomic, not just the ones that use this database. This essentially adds 2 unnecessary database queries per request. The edxapp database can probably handle this – it fields 100x more requests per second than the blockstore database does. But the blockstore database is small, and runs on shared architecture, and so if each edxapp view creates a transaction on the blockstore database, it'll fall over.

But we need the blockstore views to be atomic to ensure data integrity. So, before we enable blockstore-as-an-app in edx.org production, we need to wrap each of the Content Library REST API views which use the Blockstore API views with the atomic decorator as shown here.

This change is a data integrity change only, and should not visibly affect the user experience.

Supporting information

See:

• [BD-14] Updates Blockstore Python API [TNL-8746] [FAL-2702] openedx-unsupported/blockstore#149 (comment)
• Slack comments starting here

Testing instructions

Sandbox:

• LMS: https://pr30456.sandbox.opencraft.hosting/
• Studio: https://studio.pr30456.sandbox.opencraft.hosting/

This change is covered by the updated tests, but it's a good idea to ensure that Content Libraries still work as expected.

1. Login to the LMS as a staff member (either staff@example.com / edx or edx@example.com / edx will work)
2. Login to Studio, and visit the Libraries tab.
3. Add a new content library.
4. Add an block, and upload a file.
5. Save/Publish the block, and ensure this works as expected.

To see the collections/bundles/drafts created, you can:

1. Login to the Django Admin as a superuser (edx@example.com / edx)
2. Visit the Blockstore Bundles section and browse around.

Deadline

ASAP -- we need this change in place before we start using the blockstore app in edx.org production.

Other information

This change should also be backported to Nutmeg.

FYI @ormsbee @symbolist @bszabo

[openedx-webhooks]

Thanks for the pull request, @pomegranited! I've created BLENDED-1235 to keep track of it in Jira. More details are on the BD-14 project page.

When this pull request is ready, tag your edX technical lead.

[pomegranited]

Hi @arbrandes , can you help us with a question here?

@farhaanbukhsh is reviewing this internally and found that he couldn't add Libraries v2 content to Randomized Content Problems. And sure enough, digging into the code, it looks like the LibraryTools service only supports modulestore libraries content.

Is there some switch somewhere we need to flip to test that Libraries v2 blocks still work in these types of problems?

[farhaanbukhsh]

👍

Apart from Jill's comment, everything is working very smoothly :)

• ✅ I tested this: setting up a local devstack and uploading a file.
• ✅ I read through the code
• ❌ I checked for accessibility issues
• ❌ Includes documentation
• ❌ I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

[pomegranited]

Cool, thank you @farhaanbukhsh ! Who do we ping for upstream review on this?

[arbrandes]

@pomegranited, @farhaanbukhsh:

Is there some switch somewhere we need to flip to test that Libraries v2 blocks still work in these types of problems?

AFAIK, inclusion of Libraries v2 content in a course has only ever worked via the "source from library" block. Unless improvements have been made without my knowledge, I don't expect blockstore content to work with any of the randomized content options currently in edx-platform.

[pomegranited]

@arbrandes Thank you so much for your reply! I didn't know about the library_sourced blocks, but was able to add this block type on the sandbox course and use the library without issue.

@farhaanbukhsh if you can confirm, then this PR is definitely ready for upstream review.

[farhaanbukhsh]

@pomegranited @arbrandes Thanks a lot for the insight :)

if you can confirm, then this PR is definitely ready for upstream review.

This is good for the upstream review :)

[bradenmacdonald]

Looks good, just had a couple questions.

I read on Slack that the original plan was to use @transaction.atomic(using=BLOCKSTORE_DB_ALIAS) . Is there a reason that it's better to use @transaction.atomic by itself as done here?

---

I tested this on the sandbox and everything in Studio looked good, but I can't seem to see the course in the LMS as https://learning.pr30456.sandbox.opencraft.hosting/ is not loading (domain name not found)

[pomegranited]

Thanks for your review @bradenmacdonald !

I read on Slack that the original plan was to use @transaction.atomic(using=BLOCKSTORE_DB_ALIAS) . Is there a reason that it's better to use @transaction.atomic by itself as done here?

Yes -- we want these content library views to be atomic regardless of which database we're using, be it the default shared database, or the separately-configured blockstore database.

I tested this on the sandbox and everything in Studio looked good, but I can't seem to see the course in the LMS as https://learning.pr30456.sandbox.opencraft.hosting/ is not loading (domain name not found)

Ah oops, thanks for spotting that. I've reprovisioned so that DNS entry has been added, and this link works now:

https://learning.pr30456.sandbox.opencraft.hosting/course/course-v1:newcourse+newcow+cou1/home

[bradenmacdonald]

@pomegranited Thanks for that change, seems good now!

There are still some weird issues with the LMS on the Sandbox (the learning domain is not appearing for me) but I don't think they're related to this PR. I don't see any blockstore-related errors and the basic library functionality seems to be working well.

[openedx-webhooks]

@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.