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

[DO NOT MERGE] Improve the lock management #3

Closed
wants to merge 4 commits into from
Closed

Conversation

ashetkar
Copy link
Collaborator

@ashetkar ashetkar commented Nov 26, 2024

  • Ensure only the owner unlocks the lock
    • There is a possibility that another thread might set the locked column to false in the YB_FLYWAY_LOCK_TABLE table even if it didn't set it to true earlier.
    • To avoid this, changed column locked boolean to lock_id bigint and checking its value while resetting it.
  • Ensure the lock is not held indefinitely
    • Added a timestamp column in the lock table
    • If this (current_time - timestamp) is more than a specified ttl value (default 60000ms and configurable via a system property), lock is reset.
  • Tested the changes with existing tests in flyway-tests
  • Once reviewed, this PR will be raised against the upstream master.

@ashetkar ashetkar changed the title [DO NOT MERGE] Ensure that only the thread which sets the lock_id, resets it [DO NOT MERGE] Improve the lock management Nov 28, 2024
@kneeraj
Copy link

kneeraj commented Dec 2, 2024

Did an offline review with Amogh:
Gave the following suggestions which are now taken care of. So approving.

Suggestions given:

  1. There was a blind drop of lock table in the initialisation code which would have failed an ongoing migration. A concurrency issue.
  2. Advised to use random long for id in order to make the clash of id used for uniqueness negligible. It was int earlier.
  3. Increase the max consideration time of a migration life from 1 min to 5 min. In fact have advised to test with large migrations in a real distributed setup as migrations are real ddl executions and a big one can take a lot of time.
  4. Put some code comments around the logic.

@ashetkar
Copy link
Collaborator Author

The PR is raised with the upstream repo flyway#76

@ashetkar ashetkar closed this Jan 30, 2025
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