feat(common): #RBACK-205, vertx lock on startup scripts#958
feat(common): #RBACK-205, vertx lock on startup scripts#958
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a distributed locking mechanism using Vert.x shared data to prevent concurrent execution of SQL migration scripts during application startup. This ensures that only one instance processes database migrations at a time in a clustered environment.
Changes:
- Added configurable timeout and delay constants for SQL lock management
- Wrapped SQL migration execution with a Vert.x distributed lock acquisition and release mechanism
- Enhanced error handling and logging for lock acquisition failures
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sharedData.releaseLockAfterDelay(lock, SQL_LOCK_RELEASE_DELAY); | ||
| if (result.failed()) { |
There was a problem hiding this comment.
The lock is released even when migration fails. If a migration fails partway through, releasing the lock allows another instance to potentially run migrations on a database in an inconsistent state. Consider only releasing the lock on successful completion, or implementing a retry mechanism with appropriate failure handling.
| sharedData.releaseLockAfterDelay(lock, SQL_LOCK_RELEASE_DELAY); | |
| if (result.failed()) { | |
| if (result.succeeded()) { | |
| sharedData.releaseLockAfterDelay(lock, SQL_LOCK_RELEASE_DELAY); | |
| } else { |
| final SharedDataHelper sharedData = SharedDataHelper.getInstance(); | ||
| final String lockName = "sql_migration_" + schema; | ||
| sharedData.getLock(lockName, SQL_LOCK_TIMEOUT) | ||
| .onFailure(th -> log.debug("Lock '" + lockName + "' unavailable, another instance is processing SQL scripts", th)) |
There was a problem hiding this comment.
When lock acquisition fails, the application continues without running migrations, which could lead to the application starting with an outdated database schema. This should either fail the startup process or implement a retry mechanism to ensure migrations are eventually executed.
| final SharedDataHelper sharedData = SharedDataHelper.getInstance(); | ||
| final String lockName = "sql_migration_" + schema; | ||
| sharedData.getLock(lockName, SQL_LOCK_TIMEOUT) | ||
| .onFailure(th -> log.debug("Lock '" + lockName + "' unavailable, another instance is processing SQL scripts", th)) |
There was a problem hiding this comment.
Lock acquisition failure is logged at debug level, making it easy to miss in production environments. This should be logged at a higher level (warn or error) since it represents a significant operational event that affects application startup behavior.
| .onFailure(th -> log.debug("Lock '" + lockName + "' unavailable, another instance is processing SQL scripts", th)) | |
| .onFailure(th -> log.warn("Lock '" + lockName + "' unavailable, another instance is processing SQL scripts", th)) |
|
jenkinsEdificePublic
force-pushed
the
dev
branch
from
03ba895 to
145f82a
Compare
benjaminperez
force-pushed
the
dev
branch
2 times, most recently
from
8d46c59 to
e054ba3
Compare
Description
Add a vertx lock on startup scripts (migrations...).
Fixes
#RBACK-205
Type of change
Please check options that are relevant.
Which packages changed?
Please check the name of the package you changed
Tests
Reminder
Security flaws
Performance impacts (think bulk !)
Unit tests were replayed
Unit tests were added and/or changed
I have updated the reminder for the version including my modifications
All done ! 😃