Fix: Implement concurrency control for custom property updates in TypeRepository#25961
Fix: Implement concurrency control for custom property updates in TypeRepository#25961aniketkatkar97 wants to merge 3 commits intomainfrom
Conversation
| private static final ConcurrentHashMap<UUID, Object> TYPE_PROPERTY_LOCKS = | ||
| new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
💡 Quality: Lock map entries not cleaned up on type deletion
The TYPE_PROPERTY_LOCKS ConcurrentHashMap accumulates entries for every type UUID that has addCustomProperty called on it, but entries are never removed — not even when a type is deleted via postDelete. While the number of types in practice is small (O(100s)), this is a minor resource leak that goes against the principle of cleaning up resources.
The postDelete method already exists and handles TypeRegistry cleanup, making it the natural place to also clean up the lock entry.
Suggested fix:
@Override
protected void postDelete(Type entity, boolean hardDelete) {
super.postDelete(entity, hardDelete);
TypeRegistry.instance().removeType(entity.getName());
TYPE_PROPERTY_LOCKS.remove(entity.getId());
}
Was this helpful? React with 👍 / 👎
| Object lock = TYPE_PROPERTY_LOCKS.computeIfAbsent(id, k -> new Object()); | ||
| synchronized (lock) { |
There was a problem hiding this comment.
💡 Edge Case: JVM-level lock won't protect multi-instance deployments
The synchronized block with a ConcurrentHashMap-based lock only provides thread-safety within a single JVM. If OpenMetadata is deployed with multiple application instances (e.g., behind a load balancer), concurrent addCustomProperty requests routed to different instances will still race without coordination.
For single-instance deployments, this fix is correct and sufficient. For multi-instance scenarios, a database-level optimistic locking strategy (e.g., using the entity version/ETag) or a distributed lock would be needed. This may be worth documenting as a known limitation in a code comment or tracking as a follow-up.
Was this helpful? React with 👍 / 👎
🔍 CI failure analysis for 51b0526: All CI failures are unrelated to this PR's TypeRepository changes. New failures: 6 consistent DataProductResourceTest failures (bulk operations, migration, domain assignment) in both maven-sonarcloud-ci and Test Report. Other recurring failures include GitHub Actions auth, OpenSearch connection pools, and Python pytest segfault.IssueMultiple CI jobs failed with infrastructure and test issues unrelated to this PR's changes:
Root CauseThis PR modifies only:
All failures are unrelated to these Type concurrency control changes: 1. Previous Failures (still applicable)
2. maven-sonarcloud-ci Failure (job 64019652202)6 failures in DataProductResourceTest.java:
Result: 7948 tests run, 6 failures, 701 skipped 3. Test Report Failure (job 64045278814)Same 6 failures in DataProductResourceTest - Identical to maven-sonarcloud-ci failures DetailsAll test failures involve DataProduct functionality completely separate from Type metadata:
Pattern: The same 6 DataProductResourceTest failures appear consistently in both maven-sonarcloud-ci and Test Report jobs. These are failures in Data Product functionality, completely separate from the Type metadata repository concurrency control changes introduced by this PR. The failures suggest:
SummaryAll CI failures are unrelated to this PR's Java TypeRepository concurrency control changes:
None of these failures can be addressed by changes to this PR's TypeRepository code. They represent pre-existing issues in other areas of the codebase and CI infrastructure. Code Review 👍 Approved with suggestions 0 resolved / 2 findingsSolid implementation of per-type JVM-level locking to fix the read-modify-write race condition. Two prior minor findings remain: lock map entries aren't cleaned up on type deletion, and the JVM-level lock doesn't protect multi-instance deployments — both are acknowledged limitations with low practical impact. 💡 Quality: Lock map entries not cleaned up on type deletion📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java:68-69 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java:148-151 The The Suggested fix💡 Edge Case: JVM-level lock won't protect multi-instance deployments📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TypeRepository.java:168-169 The For single-instance deployments, this fix is correct and sufficient. For multi-instance scenarios, a database-level optimistic locking strategy (e.g., using the entity version/ETag) or a distributed lock would be needed. This may be worth documenting as a known limitation in a code comment or tracking as a follow-up. Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|



Before:
After:
This pull request introduces thread-safety improvements for adding custom properties to types and adds a corresponding concurrent test to ensure data integrity under concurrent operations. The main focus is to prevent race conditions when multiple threads attempt to add custom properties to the same type simultaneously.
Concurrency control for custom property updates:
ConcurrentHashMap(TYPE_PROPERTY_LOCKS) inTypeRepository.javato provide per-type locking, ensuring that concurrent additions of custom properties to the same type are synchronized and do not cause data loss or corruption. [1] [2]addCustomPropertymethod inTypeRepository.javato acquire a lock specific to the type's UUID before modifying its custom properties, preventing race conditions during concurrent updates. [1] [2]Testing and validation:
put_concurrent_customProperty_sameType_allPersistedinTypeResourceTest.javathat uses multiple threads to concurrently add custom properties to the same type and verifies that all properties are persisted without errors or data loss. [1] [2]