Fix Concurrency Issues and Add Nitrite Integration Tests#2308
Open
jpgough-ms wants to merge 10 commits intofinos:mainfrom
Open
Fix Concurrency Issues and Add Nitrite Integration Tests#2308jpgough-ms wants to merge 10 commits intofinos:mainfrom
jpgough-ms wants to merge 10 commits intofinos:mainfrom
Conversation
Replace check-then-act patterns with atomic operations to eliminate TOCTOU race conditions in all CalmHub store implementations. MongoDB stores: - Use unique indexes + try-insertOne/catch-DuplicateKey for namespace, domain, and schema stores - Use atomic updateOne with $elemMatch + $exists filters for architecture, pattern, flow, standard, interface, and control stores - Add MongoIndexInitializer to create unique indexes at startup Nitrite stores: - Add ReentrantLock (via Lock interface) with lock/try/finally to protect check-then-write in all create operations Also: - Add NamespaceAlreadyExistsException checked exception - Update NamespaceStore interface and NamespaceResource - Update all affected unit tests Refs: finos#2284
Add integration test suite for Nitrite storage backend covering all resource types: ADR, architecture, control, decorator, domain, flow, interface, namespace, pattern, and standard. Fix concurrency issues in NitriteInterfaceStore, NitritePatternStore, and NitriteStandardStore with corresponding unit test updates.
…tegration tests Add locking to 8 Nitrite store create methods to prevent data loss under concurrent writes. Add concurrency integration tests (20 threads) for all entity types across both Nitrite and MongoDB backends. Stores fixed: Pattern, Architecture, Flow, Standard, Interface, Decorator, Control (requirements + configurations), and ADR.
# Conflicts: # calm-hub/src/main/java/org/finos/calm/store/mongo/MongoInterfaceStore.java # calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteInterfaceStore.java
Replace synchronized keyword in NitriteCounterStore with ReentrantLock and add missing lock protection in NitriteUserAccessStore to ensure consistent concurrency control across all Nitrite store implementations.
…ore conferences folder Add comprehensive Javadoc comments to all MongoDB and Nitrite store classes explaining the concurrency control strategies: - MongoIndexInitializer: startup index creation lifecycle - Namespace/Domain stores: optimistic insert with DUPLICATE_KEY handling - Architecture/Pattern/Flow/Standard/Interface stores: upsert+push pattern - ControlStore: nested arrayFilters for configuration versions - CounterStore: atomic findOneAndUpdate with $inc - CoreSchemaStore: idempotent create with silent duplicate handling - Nitrite stores: ReentrantLock strategy for standalone mode Restore accidentally deleted conferences/osff-ln-2025 workshop folder.
Member
Author
|
At this stage I've decided to leave the concurrency tests as part of the integration suite, they only add 16 seconds to the 1:20 build time on my Mac
|
jpgough-ms
requested review from
Thels,
grahampacker-ms,
markscott-ms and
rocketstack-matt
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens CALM Hub’s persistence layer against concurrent writes by making MongoDB operations atomic (and backed by unique indexes) and by serializing Nitrite (standalone) writes with JVM-level locks, alongside expanded integration and unit test coverage for concurrency/idempotency behavior.
Changes:
- Add MongoDB unique-index initialization at startup and update stores to rely on atomic conditional updates / duplicate-key handling for concurrency safety.
- Add
ReentrantLock-based write serialization to Nitrite stores and adjust Nitrite version handling to return dotted semantic versions. - Update unit tests and add end-to-end + concurrency integration tests for both MongoDB and Nitrite backends.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoIndexInitializer.java | New startup initializer that ensures unique indexes needed for safe concurrent inserts/updates. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoNamespaceStore.java | Translate Mongo duplicate-key errors into NamespaceAlreadyExistsException (optimistic insert). |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoDomainStore.java | Translate Mongo duplicate-key errors into DomainAlreadyExistsException (optimistic insert). |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoCoreSchemaStore.java | Make schema version creation idempotent by ignoring duplicate-key inserts. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoArchitectureStore.java | Use atomic conditional updates for version creation under concurrency. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoPatternStore.java | Use atomic conditional updates for version creation under concurrency. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoFlowStore.java | Use atomic conditional updates for version creation under concurrency. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoStandardStore.java | Use atomic conditional updates for version creation under concurrency. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoInterfaceStore.java | Use atomic conditional updates for version creation under concurrency. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoControlStore.java | Use atomic conditional updates (incl. array filters) for nested version creation under concurrency. |
| calm-hub/src/main/java/org/finos/calm/store/mongo/MongoCounterStore.java | Add documentation clarifying atomic counter behavior for ID generation. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteCounterStore.java | Replace synchronized counter increment with explicit lock. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteNamespaceStore.java | Add lock + NamespaceAlreadyExistsException to prevent duplicate namespace creation in Nitrite. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteDomainStore.java | Add lock + duplicate prevention for domain creation in Nitrite. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteStandardStore.java | Add locks to creation/versioning; normalize returned version strings to dotted format. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteInterfaceStore.java | Add locks to creation/versioning; normalize returned version strings to dotted format. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitritePatternStore.java | Add locks; normalize returned version lists to dotted format. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteFlowStore.java | Add locks around ID generation and version creation to avoid races. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteArchitectureStore.java | Add locks around ID generation and version creation to avoid races. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteDecoratorStore.java | Add lock around decorator creation to avoid ID/list races. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteAdrStore.java | Add lock around ADR creation to avoid ID/list races. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteUserAccessStore.java | Add lock around user-access creation to avoid ID/insert races. |
| calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteControlStore.java | Add lock-based serialization for control/config/version writes. |
| calm-hub/src/main/java/org/finos/calm/store/NamespaceStore.java | Update contract to throw NamespaceAlreadyExistsException on create. |
| calm-hub/src/main/java/org/finos/calm/resources/NamespaceResource.java | Handle NamespaceAlreadyExistsException and return HTTP 409 without racy pre-check. |
| calm-hub/src/main/java/org/finos/calm/domain/exception/NamespaceAlreadyExistsException.java | New domain exception for duplicate namespace creation. |
| calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoNamespaceStoreShould.java | Update tests for duplicate-key behavior (but currently contains an unused import). |
| calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoDomainStoreShould.java | Update tests to simulate duplicate-key behavior via Mongo exceptions. |
| calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoCoreSchemaStoreShould.java | Update tests for idempotent duplicate-key handling. |
| calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongo*StoreShould.java | Update version-create tests to stub UpdateResult for new atomic update logic. |
| calm-hub/src/test/java/org/finos/calm/store/nitrite/TestNitrite*StoreShould.java | Update tests to expect dotted semantic versions and string-based version payloads. |
| calm-hub/src/test/java/org/finos/calm/resources/TestNamespaceResourceShould.java | Update resource tests to expect exception-driven 409 behavior (no pre-check). |
| calm-hub/src/integration-test/java/integration/performance/ConcurrencyTestHelper.java | New concurrency harness for integration tests. |
| calm-hub/src/integration-test/java/integration/performance/MongoConcurrencyIntegration.java | New MongoDB concurrency integration tests across entities and version creation. |
| calm-hub/src/integration-test/java/integration/performance/NitriteConcurrencyIntegration.java | New Nitrite concurrency integration tests across entities and version creation. |
| calm-hub/src/integration-test/java/integration/NitriteIntegration.java | New Nitrite end-to-end integration tests for multiple resources. |
| calm-hub/src/integration-test/java/integration/NitriteEndToEndResource.java | New Quarkus test resource for isolated Nitrite DB directory per test run. |
| calm-hub/src/integration-test/java/integration/NitriteIntegrationTestProfile.java | New Quarkus test profile for Nitrite integration tests. |
| calm-hub/src/integration-test/java/integration/MongoSetup.java | Simplify Mongo test setup by checking countDocuments() instead of creating collections manually. |
| calm-hub/src/integration-test/java/integration/MongoSchemaIntegration.java | Simplify schema setup by inserting only when collection is empty. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
calm-hub/src/test/java/org/finos/calm/store/mongo/TestMongoNamespaceStoreShould.java
Outdated
Show resolved
Hide resolved
calm-hub/src/main/java/org/finos/calm/store/nitrite/NitritePatternStore.java
Show resolved
Hide resolved
calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteControlStore.java
Outdated
Show resolved
Hide resolved
calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteControlStore.java
Outdated
Show resolved
Hide resolved
calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteControlStore.java
Show resolved
Hide resolved
…methods Three write methods (createRequirementForVersion, createControlConfiguration, createConfigurationForVersion) fetched domainDoc twice - once via findControl() and again before update - causing mutations to be lost. Refactored to fetch domainDoc once, mutate nested documents in place, and update with the same reference. Added findControlInDomainDoc and findConfigurationInControlDoc helpers. Also removed unused ErrorCategory import from TestMongoNamespaceStoreShould.
jpgough-ms
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Commit Message Format ✅
Testing
Checklist