Test to highlight ModuleSetTag Inefficiencies

- Add (micrometer) instrumentation to expose inefficiencies
- Add test config for micrometer
- Add setup methods in base to create many cm handles
- Set module sync parallelism to 2 for testing
- Add clean up methods for hazelcast related tests
- added test to show inefficiencies
- POC 1 use hazelcast set to prevent multiple threads working on same ModuleSetTag
- POC 2 'cache' module set tags per thread to prevent DB looks ups
- Main inefficiency left: create schemaset for EACH cm Handled even if same tag. No easy PoC...

Change-Id: Idf46b44c475a24727dd7084bb613459f4c29be55
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>

4 files changed, 105 insertions, 16 deletions

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/cache/HazelcastCacheConfigSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/cache/HazelcastCacheConfigSpec.groovy
index 0bd838437d..c08ff75a44 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/cache/HazelcastCacheConfigSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/cache/HazelcastCacheConfigSpec.groovy
@@ -22,12 +22,17 @@ package org.onap.cps.ncmp.impl.cache
 
 import com.hazelcast.config.Config
 import com.hazelcast.config.RestEndpointGroup
+import com.hazelcast.core.Hazelcast
 import spock.lang.Specification
 
 class HazelcastCacheConfigSpec extends Specification {
 
     def objectUnderTest = new HazelcastCacheConfig()
 
+    def cleanupSpec() {
+        Hazelcast.getHazelcastInstanceByName('my instance config').shutdown()
+    }
+
     def 'Create Hazelcast instance with a #scenario'() {
         given: 'a cluster name and instance config name'
             objectUnderTest.clusterName = 'my cluster'
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncServiceSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncServiceSpec.groovy
index 6030e5debf..2f13a9a483 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncServiceSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncServiceSpec.groovy
@@ -20,14 +20,17 @@
 
 package org.onap.cps.ncmp.impl.inventory.sync
 
+import com.hazelcast.collection.ISet
 import org.onap.cps.api.CpsAnchorService
 import org.onap.cps.api.CpsDataService
 import org.onap.cps.api.CpsModuleService
+import org.onap.cps.ncmp.api.exceptions.NcmpException
 import org.onap.cps.ncmp.api.inventory.models.CompositeStateBuilder
 import org.onap.cps.ncmp.api.inventory.models.NcmpServiceCmHandle
 import org.onap.cps.ncmp.impl.inventory.CmHandleQueryService
 import org.onap.cps.ncmp.impl.inventory.models.CmHandleState
 import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle
+import org.onap.cps.ncmp.impl.inventory.sync.ModuleSyncService.ModuleDelta
 import org.onap.cps.spi.CascadeDeleteAllowed
 import org.onap.cps.spi.exceptions.SchemaSetNotFoundException
 import org.onap.cps.spi.model.ModuleReference
@@ -45,18 +48,22 @@ class ModuleSyncServiceSpec extends Specification {
     def mockCmHandleQueries = Mock(CmHandleQueryService)
     def mockCpsDataService = Mock(CpsDataService)
     def mockJsonObjectMapper = Mock(JsonObjectMapper)
+    def mockModuleSetTagsBeingProcessed = Mock(ISet<String>);
 
-    def objectUnderTest = new ModuleSyncService(mockDmiModelOperations, mockCpsModuleService,
-            mockCpsDataService, mockCpsAnchorService, mockJsonObjectMapper)
+    def objectUnderTest = new ModuleSyncService(mockDmiModelOperations, mockCpsModuleService, mockCpsDataService, mockCpsAnchorService, mockJsonObjectMapper, mockModuleSetTagsBeingProcessed)
 
     def expectedDataspaceName = NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME
 
-    def 'Sync model for a NEW cm handle using module set tags: #scenario.'() {
-        given: 'a cm handle state to be synced'
-            def ncmpServiceCmHandle = new NcmpServiceCmHandle()
-            ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder().withCmHandleState(CmHandleState.ADVISED).build())
-            ncmpServiceCmHandle.cmHandleId = 'ch-1'
-            def yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle('some service name', '', '', ncmpServiceCmHandle, moduleSetTag, '', '')
+    def setup() {
+        // Allow tags for al test except 'duplicate-processing-tag' to be added to processing semaphore
+        mockModuleSetTagsBeingProcessed.add('new-tag') >> true
+        mockModuleSetTagsBeingProcessed.add('same-tag') >> true
+        mockModuleSetTagsBeingProcessed.add('cached-tag') >> true
+    }
+
+    def 'Sync models for a NEW cm handle using module set tags: #scenario.'() {
+        given: 'a cm handle to be synced'
+            def yangModelCmHandle = createAdvisedCmHandle(moduleSetTag)
         and: 'DMI operations returns some module references'
             def moduleReferences =  [ new ModuleReference('module1','1'), new ModuleReference('module2','2') ]
             mockDmiModelOperations.getModuleReferences(yangModelCmHandle) >> moduleReferences
@@ -75,10 +82,60 @@ class ModuleSyncServiceSpec extends Specification {
         where: 'the following parameters are used'
             scenario                  | identifiedNewModuleReferences         | newModuleNameContentToMap     | moduleSetTag | existingModuleReferences
             'one new module, new tag' | [new ModuleReference('module1', '1')] | [module1: 'some yang source'] | ''           | []
-            'no new module, new tag'  | []                                    | [:]                           | 'new-tag-1'  | []
+            'no new module, new tag'  | []                                    | [:]                           | 'new-tag'    | []
             'same tag'                | []                                    | [:]                           | 'same-tag'   | [new ModuleReference('module1', '1'), new ModuleReference('module2', '2')]
     }
 
+    def 'Attempt Sync models for a cm handle with exception and #scenario module set tag'() {
+        given: 'a cm handle to be synced'
+            def yangModelCmHandle = createAdvisedCmHandle(moduleSetTag)
+        and: 'the service returns a list of module references when queried with the specified attributes'
+            mockCpsModuleService.getModuleReferencesByAttribute(*_) >> [new ModuleReference('module1', '1')]
+        and: 'exception occurs when try to store result'
+            def testException = new RuntimeException('test')
+            mockCpsModuleService.createSchemaSetFromModules(*_) >> { throw testException }
+        when: 'module sync is triggered'
+            objectUnderTest.syncAndCreateSchemaSetAndAnchor(yangModelCmHandle)
+        then: 'the same exception is thrown up'
+            def exceptionThrown = thrown(Exception)
+            assert testException == exceptionThrown
+        and: 'module set tag is removed from processing semaphores only when needed'
+            expectedCallsToRemoveTag * mockModuleSetTagsBeingProcessed.remove('new-tag')
+        where: 'following module set tags are used'
+            scenario  | moduleSetTag || expectedCallsToRemoveTag
+            'with'    | 'new-tag'    || 1
+            'without' | ' '          || 0
+    }
+
+    def 'Sync models for a cm handle with previously cached module set tag.'() {
+        given: 'a cm handle to be synced'
+            def yangModelCmHandle = createAdvisedCmHandle('cached-tag')
+        and: 'The module set tag exist in the private cache'
+            def moduleReferences = [ new ModuleReference('module1','1') ]
+            def cachedModuleDelta = new ModuleDelta(moduleReferences, [:])
+            objectUnderTest.privateModuleSetCache.put('cached-tag', cachedModuleDelta)
+        when: 'module sync is triggered'
+            objectUnderTest.syncAndCreateSchemaSetAndAnchor(yangModelCmHandle)
+        then: 'create schema set from module is invoked with correct parameters'
+            1 * mockCpsModuleService.createSchemaSetFromModules(NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME, 'ch-1', [:], moduleReferences)
+        and: 'anchor is created with the correct parameters'
+            1 * mockCpsAnchorService.createAnchor(NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME, 'ch-1', 'ch-1')
+    }
+
+    def 'Attempt to sync using a module set tag already being processed by a different instance or thread.'() {
+        given: 'a cm handle to be synced'
+            def yangModelCmHandle = createAdvisedCmHandle('duplicateTag')
+        and: 'The module set tag already exist in the processing semaphore set'
+            mockModuleSetTagsBeingProcessed.add('duplicate-processing-tag') > false
+        when: 'module sync is triggered'
+            objectUnderTest.syncAndCreateSchemaSetAndAnchor(yangModelCmHandle)
+        then: 'a ncmp exception is thrown with the relevant details'
+            def exceptionThrown = thrown(NcmpException)
+            assert exceptionThrown.message.contains('duplicateTag')
+            assert exceptionThrown.details.contains('duplicateTag')
+            assert exceptionThrown.details.contains('ch-1')
+    }
+
     def 'Upgrade model for an existing cm handle with Module Set Tag where the modules are #scenario'() {
         given: 'a cm handle being upgraded to module set tag: tag-1'
             def ncmpServiceCmHandle = new NcmpServiceCmHandle()
@@ -113,7 +170,7 @@ class ModuleSyncServiceSpec extends Specification {
             'in database' | [new ModuleReference('module1', '1')]
     }
 
-    def 'upgrade model for a existing cm handle'() {
+    def 'upgrade model for an existing cm handle'() {
         given: 'a cm handle that is ready but locked for upgrade'
             def ncmpServiceCmHandle = new NcmpServiceCmHandle()
             ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder()
@@ -159,4 +216,20 @@ class ModuleSyncServiceSpec extends Specification {
             result == unsupportedOperationException
     }
 
+    def 'Clear module set cache.'() {
+        given: 'something in the module set cache'
+            objectUnderTest.privateModuleSetCache.put('test',new ModuleDelta([],[:]))
+        when: 'the cache is cleared'
+            objectUnderTest.clearPrivateModuleSetCache()
+        then: 'the cache is empty'
+            objectUnderTest.privateModuleSetCache.isEmpty()
+    }
+
+    def createAdvisedCmHandle(moduleSetTag) {
+        def ncmpServiceCmHandle = new NcmpServiceCmHandle()
+        ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder().withCmHandleState(CmHandleState.ADVISED).build())
+        ncmpServiceCmHandle.cmHandleId = 'ch-1'
+        return YangModelCmHandle.toYangModelCmHandle('some service name', '', '', ncmpServiceCmHandle, moduleSetTag, '', '')
+    }
+
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy
index 8ce1e934f2..e21c868bbf 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy
@@ -26,6 +26,7 @@ import ch.qos.logback.classic.Logger
 import ch.qos.logback.classic.spi.ILoggingEvent
 import ch.qos.logback.core.read.ListAppender
 import com.hazelcast.config.Config
+import com.hazelcast.core.Hazelcast
 import com.hazelcast.instance.impl.HazelcastInstanceFactory
 import com.hazelcast.map.IMap
 import org.onap.cps.ncmp.api.inventory.models.CompositeState
@@ -75,6 +76,10 @@ class ModuleSyncTasksSpec extends Specification {
     def objectUnderTest = new ModuleSyncTasks(mockInventoryPersistence, mockSyncUtils, mockModuleSyncService,
             mockLcmEventsCmHandleStateHandler, moduleSyncStartedOnCmHandles)
 
+    def cleanupSpec() {
+        Hazelcast.getHazelcastInstanceByName('hazelcastInstanceName').shutdown()
+    }
+
     def 'Module Sync ADVISED cm handles.'() {
         given: 'cm handles in an ADVISED state'
             def cmHandle1 = cmHandleAsDataNodeByIdAndState('cm-handle-1', CmHandleState.ADVISED)
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/SynchronizationCacheConfigSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/SynchronizationCacheConfigSpec.groovy
index 4c96d6b822..c2ecf927c8 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/SynchronizationCacheConfigSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/SynchronizationCacheConfigSpec.groovy
@@ -20,6 +20,7 @@
 
 package org.onap.cps.ncmp.impl.inventory.sync
 
+import com.hazelcast.collection.ISet
 import com.hazelcast.config.Config
 import com.hazelcast.core.Hazelcast
 import com.hazelcast.map.IMap
@@ -38,13 +39,16 @@ import java.util.concurrent.TimeUnit
 class SynchronizationCacheConfigSpec extends Specification {
 
     @Autowired
-    private BlockingQueue<DataNode> moduleSyncWorkQueue
+    BlockingQueue<DataNode> moduleSyncWorkQueue
 
     @Autowired
-    private IMap<String, Object> moduleSyncStartedOnCmHandles
+    IMap<String, Object> moduleSyncStartedOnCmHandles
 
     @Autowired
-    private IMap<String, Boolean> dataSyncSemaphores
+    IMap<String, Boolean> dataSyncSemaphores
+
+    @Autowired
+    ISet<String> moduleSetTagsBeingProcessed
 
     def cleanupSpec() {
         Hazelcast.getHazelcastInstanceByName('cps-and-ncmp-hazelcast-instance-test-config').shutdown()
@@ -57,8 +61,11 @@ class SynchronizationCacheConfigSpec extends Specification {
             assert null != moduleSyncStartedOnCmHandles
         and: 'system is able to create an instance of a map to hold data sync semaphores'
             assert null != dataSyncSemaphores
-        and: 'they have the correct names (in any order)'
-            assert Hazelcast.allHazelcastInstances.name.contains('cps-and-ncmp-hazelcast-instance-test-config')
+        and: 'system is able to create an instance of a set to hold module set tags being processed'
+            assert null != moduleSetTagsBeingProcessed
+        and: 'there is only one instance with the correct name'
+            assert Hazelcast.allHazelcastInstances.size() == 1
+            assert Hazelcast.allHazelcastInstances.name[0] == 'cps-and-ncmp-hazelcast-instance-test-config'
     }
 
     def 'Verify configs for Distributed objects'(){
@@ -103,7 +110,6 @@ class SynchronizationCacheConfigSpec extends Specification {
         then: 'applied properties are reflected'
             assert testConfig.networkConfig.join.kubernetesConfig.enabled
             assert testConfig.networkConfig.join.kubernetesConfig.properties.get('service-name') == 'test-service-name'
-
     }
 
     def 'Time to Live Verify for Module Sync Semaphore'() {