Skip to content

Make the migration lock atomic#479

Open
Chee7ah wants to merge 1 commit intoseppevs:masterfrom
Chee7ah:fix/atomic-lock
Open

Make the migration lock atomic#479
Chee7ah wants to merge 1 commit intoseppevs:masterfrom
Chee7ah:fix/atomic-lock

Conversation

@Chee7ah
Copy link
Copy Markdown

@Chee7ah Chee7ah commented Mar 2, 2026

Description

This is a follow up to the locking feature from #262 and intends to address the issue pointed out in #262 (comment).

Between checking whether the lock exists and placing the lock, another process could place a lock, which is currently not accounted for. As long as a single process is running the migrations, it is not an issue, but if it is part of multiple processes, we have a race condition.

As for the implementation, the lock document is explicitly inserted with an _id set. This is because upsert operations are still done in two distinct phases on the database side and could end up with duplicate values, unless there is a unique index to prevent this.

If we end up in such a tight race condition between two processes, then because of the unique index already present on the _id field, we'll end up with the DuplicateKey MongoDB error, which is the same case as when the upsert ends up not creating a new document: the lock already exists.


Tests are updated to account for the updated implementation of the locking mechanism and also included a new section in the README about the locks, as it didn't really mention how to enable them before.

+ Added a missing await to the TTL index creation to ensure that the index is ready by the time we try to add the lock to it.

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Dependency update
  • Other (please describe):

Testing

Checklist

  • My code follows the code style of this project (npm run lint passes)
  • All tests pass (npm test passes)
  • Code coverage remains at 100%
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation (if applicable)
  • I have updated the CHANGELOG.md (for user-facing changes)
  • My changes generate no new warnings
  • I have checked my code for potential security issues

Screenshots (if applicable)

Additional Notes

Copy link
Copy Markdown

@tjann tjann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants