Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent duplicate articles_courses rows. #5956

Open
gabina opened this issue Sep 10, 2024 · 0 comments
Open

Prevent duplicate articles_courses rows. #5956

gabina opened this issue Sep 10, 2024 · 0 comments
Labels

Comments

@gabina
Copy link
Member

gabina commented Sep 10, 2024

What is happening?

While we're using sidekiq-unique-jobs gem to add unique constraints to sidekiq jobs, and we currently set sidekiq_options lock: :until_executed in CourseDataUpdateWorker worker to avoid duplicate jobs (with the same arguments and job class), it's still possible to end up with duplicate articles_courses rows.
One possible scenario for this is when running a manual update course stats process through the manual_update route. As this update runs synchronously there is no guarantee that a queued UpdateCourseStats job will run simultaneously.

To Reproduce

Steps to reproduce the behavior:
As the problem is related to a race condition, it's not trivial to reproduce. However, this is one scenario that may show the problem:

  1. Start the app locally.
  2. Be sure sidekiq is up (bundle exec sidekiq start).
  3. Download a course locally (you can do this through the "Copy course from another server" admin view).
  4. Add the course to default campaign.
  5. Run a manual update using the manual_update route (you need to be an admin for that).

If an update for that course was scheduled and run at the same time than your synchronous update, then duplicate articles_courses rows may be created.

Expected behavior

We should avoid having duplicate articles_courses rows (i.e., multiple rows with the same course_id and article_id). This can be prevented by adding a unique constraint on (course_id, article_id) in the articles_courses table. Proper tests should be done before migrating articles_courses table.

@gabina gabina added the bug label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant