Performance Improvement: Use save batches of cmhandles 77/130677/2
authorsourabh_sourabh <sourabh.sourabh@est.tech>
Fri, 2 Sep 2022 08:40:35 +0000 (09:40 +0100)
committersourabh_sourabh <sourabh.sourabh@est.tech>
Fri, 2 Sep 2022 14:29:00 +0000 (15:29 +0100)
-Used cm handle batch to persist from state handler.

Issue-ID: CPS-1230
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>
Change-Id: I68b7fde7dc85818b818f1af588344c26b549d87b

cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasks.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy

index c21c74b..e8a6411 100755 (executable)
@@ -300,14 +300,18 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
     public List<CmHandleRegistrationResponse> parseAndCreateCmHandlesInDmiRegistrationAndSyncModules(
             final DmiPluginRegistration dmiPluginRegistration) {
         List<CmHandleRegistrationResponse> cmHandleRegistrationResponses = new ArrayList<>();
+        final Map<YangModelCmHandle, CmHandleState> cmHandleStatePerCmHandle = new HashMap<>();
         try {
-            cmHandleRegistrationResponses = dmiPluginRegistration.getCreatedCmHandles().stream()
-                    .map(cmHandle ->
-                            YangModelCmHandle.toYangModelCmHandle(
-                                    dmiPluginRegistration.getDmiPlugin(),
-                                    dmiPluginRegistration.getDmiDataPlugin(),
-                                    dmiPluginRegistration.getDmiModelPlugin(),
-                                    cmHandle)).map(this::registerNewCmHandle).collect(Collectors.toList());
+            dmiPluginRegistration.getCreatedCmHandles()
+                    .forEach(cmHandle -> {
+                        final YangModelCmHandle yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle(
+                                dmiPluginRegistration.getDmiPlugin(),
+                                dmiPluginRegistration.getDmiDataPlugin(),
+                                dmiPluginRegistration.getDmiModelPlugin(),
+                                cmHandle);
+                        cmHandleStatePerCmHandle.put(yangModelCmHandle, CmHandleState.ADVISED);
+                    });
+            cmHandleRegistrationResponses = registerNewCmHandles(cmHandleStatePerCmHandle);
         } catch (final DataValidationException dataValidationException) {
             cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createFailureResponse(dmiPluginRegistration
                             .getCreatedCmHandles().stream()
@@ -356,15 +360,19 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
         inventoryPersistence.deleteListOrListElement("/dmi-registry/cm-handles[@id='" + cmHandleId + "']");
     }
 
-    private CmHandleRegistrationResponse registerNewCmHandle(final YangModelCmHandle yangModelCmHandle) {
+    private List<CmHandleRegistrationResponse> registerNewCmHandles(final Map<YangModelCmHandle, CmHandleState>
+                                                                            cmHandleStatePerCmHandle) {
+        final List<String> cmHandleIds = cmHandleStatePerCmHandle.keySet().stream().map(YangModelCmHandle::getId)
+                .collect(Collectors.toList());
         try {
-            lcmEventsCmHandleStateHandler.updateCmHandleState(yangModelCmHandle, CmHandleState.ADVISED);
-            return CmHandleRegistrationResponse.createSuccessResponse(yangModelCmHandle.getId());
+            lcmEventsCmHandleStateHandler.updateCmHandleStateBatch(cmHandleStatePerCmHandle);
+            return CmHandleRegistrationResponse.createSuccessResponses(cmHandleIds);
         } catch (final AlreadyDefinedException alreadyDefinedException) {
-            return CmHandleRegistrationResponse.createFailureResponse(
-                    yangModelCmHandle.getId(), RegistrationError.CM_HANDLE_ALREADY_EXIST);
+            return List.of(CmHandleRegistrationResponse.createFailureResponse(
+                    String.join(",", cmHandleIds), RegistrationError.CM_HANDLE_ALREADY_EXIST));
         } catch (final Exception exception) {
-            return CmHandleRegistrationResponse.createFailureResponse(yangModelCmHandle.getId(), exception);
+            return List.of(CmHandleRegistrationResponse.createFailureResponse(String.join(",", cmHandleIds),
+                    exception));
         }
     }
 }
index 597e2ba..ada3dc6 100644 (file)
@@ -78,7 +78,7 @@ public class ModuleSyncTasks {
                 }
                 log.debug("{} is now in {} state", cmHandleId, compositeState.getCmHandleState().name());
             }
-            updateCmHandlesStateBatch(cmHandelStatePerCmHandle);
+            lcmEventsCmHandleStateHandler.updateCmHandleStateBatch(cmHandelStatePerCmHandle);
         } finally {
             batchCounter.getAndDecrement();
         }
@@ -98,11 +98,11 @@ public class ModuleSyncTasks {
             final boolean isReadyForRetry = syncUtils.isReadyForRetry(compositeState);
             if (isReadyForRetry) {
                 log.debug("Reset cm handle {} state to ADVISED to be re-attempted by module-sync watchdog",
-                    failedCmHandle.getId());
+                        failedCmHandle.getId());
                 cmHandleStatePerCmHandle.put(failedCmHandle, CmHandleState.ADVISED);
             }
         }
-        updateCmHandlesStateBatch(cmHandleStatePerCmHandle);
+        lcmEventsCmHandleStateHandler.updateCmHandleStateBatch(cmHandleStatePerCmHandle);
         return COMPLETED_FUTURE;
     }
 
@@ -111,11 +111,4 @@ public class ModuleSyncTasks {
         advisedCmHandle.getCompositeState().setLockReason(lockReason);
     }
 
-    private void updateCmHandlesStateBatch(final Map<YangModelCmHandle, CmHandleState> cmHandleStatePerCmHandle) {
-        // To be refactored as part of CPS-1231; Use state-save-batch capability (depends sub-task12, 13)
-        for (final Map.Entry<YangModelCmHandle, CmHandleState> entry : cmHandleStatePerCmHandle.entrySet()) {
-            lcmEventsCmHandleStateHandler.updateCmHandleState(entry.getKey(), entry.getValue());
-        }
-    }
-
 }
index 1da2aa9..b7faf09 100644 (file)
@@ -21,6 +21,8 @@
 
 package org.onap.cps.ncmp.api.models;
 
+import java.util.List;
+import java.util.stream.Collectors;
 import lombok.Builder;
 import lombok.Data;
 import lombok.RequiredArgsConstructor;
@@ -70,6 +72,11 @@ public class CmHandleRegistrationResponse {
             .status(Status.SUCCESS).build();
     }
 
+    public static List<CmHandleRegistrationResponse> createSuccessResponses(final List<String> cmHandleIds) {
+        return cmHandleIds.stream().map(CmHandleRegistrationResponse::createSuccessResponse)
+                .collect(Collectors.toList());
+    }
+
     public enum Status {
         SUCCESS, FAILURE;
     }
index ed985ec..86a32a1 100644 (file)
@@ -159,12 +159,15 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
                 assert it.cmHandle == 'cmhandle'
             }
         and: 'state handler is invoked with the expected parameters'
-            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, _) >> {
-                args -> {
-                        def result = (args[0] as YangModelCmHandle)
-                        assert result.id == 'cmhandle'
-                        assert result.dmiServiceName == 'my-server'
-                        assert CmHandleState.ADVISED == (args[1] as CmHandleState)
+            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> {
+                args ->
+                    {
+                        def cmHandleStatePerCmHandle = (args[0] as Map)
+                        cmHandleStatePerCmHandle.each {
+                            assert (it.key.id == 'cmhandle'
+                                    && it.key.dmiServiceName == 'my-server'
+                                    && it.value == CmHandleState.ADVISED)
+                        }
                     }
             }
         where:
@@ -173,36 +176,29 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             'with only public properties'     | [:]                      | ['public-key': 'public-value'] || '[]'                                       | '[{"name":"public-key","value":"public-value"}]'
             'with only dmi properties'        | ['dmi-key': 'dmi-value'] | [:]                            || '[{"name":"dmi-key","value":"dmi-value"}]' | '[]'
             'without dmi & public properties' | [:]                      | [:]                            || '[]'                                       | '[]'
-
     }
 
-    def 'Create CM-Handle Multiple Requests: All cm-handles creation requests are processed'() {
+    def 'Create CM-Handle Multiple Requests: All cm-handles creation requests are processed with some failures'() {
         given: 'a registration with three cm-handles to be created'
             def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server',
-                createdCmHandles: [new NcmpServiceCmHandle(cmHandleId: 'cmhandle1'),
-                                   new NcmpServiceCmHandle(cmHandleId: 'cmhandle2'),
-                                   new NcmpServiceCmHandle(cmHandleId: 'cmhandle3')])
+                    createdCmHandles: [new NcmpServiceCmHandle(cmHandleId: 'cmhandle1'),
+                                       new NcmpServiceCmHandle(cmHandleId: 'cmhandle2'),
+                                       new NcmpServiceCmHandle(cmHandleId: 'cmhandle3')])
         and: 'cm-handle creation is successful for 1st and 3rd; failed for 2nd'
-            mockLcmEventsCmHandleStateHandler.updateCmHandleState(*_) >> {} >> { throw new RuntimeException("Failed") } >> {}
+            mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(*_) >> { throw new RuntimeException("Failed") }
         when: 'registration is updated to create cm-handles'
             def response = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration)
         then: 'a response is received for all cm-handles'
-            response.getCreatedCmHandles().size() == 3
-        and: '1st and 3rd cm-handle are created successfully'
+            response.getCreatedCmHandles().size() == 1
+        and: 'all cm-handles creation fails'
             with(response.getCreatedCmHandles().get(0)) {
-                assert it.status == Status.SUCCESS
-                assert it.cmHandle == 'cmhandle1'
-            }
-            with(response.getCreatedCmHandles().get(2)) {
-                assert it.status == Status.SUCCESS
-                assert it.cmHandle == 'cmhandle3'
-            }
-        and: '2nd cm-handle creation fails'
-            with(response.getCreatedCmHandles().get(1)) {
                 assert it.status == Status.FAILURE
                 assert it.registrationError == UNKNOWN_ERROR
                 assert it.errorText == 'Failed'
-                assert it.cmHandle == 'cmhandle2'
+                def sortedCmHandles = it.cmHandle.split(',').sort()
+                assert sortedCmHandles[0] == 'cmhandle1'
+                assert sortedCmHandles[1] == 'cmhandle2'
+                assert sortedCmHandles[2] == 'cmhandle3'
             }
     }
 
@@ -211,7 +207,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server')
             dmiPluginRegistration.createdCmHandles = [new NcmpServiceCmHandle(cmHandleId: cmHandleId)]
         and: 'cm-handler registration fails: #scenario'
-            mockLcmEventsCmHandleStateHandler.updateCmHandleState(*_) >> { throw exception }
+            mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(*_) >> { throw exception }
         when: 'registration is updated'
             def response = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration)
         then: 'a failure response is received'
index 02cfb15..def0db3 100644 (file)
@@ -277,17 +277,20 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
     def 'Verify modules and create anchor params'() {
         given: 'dmi plugin registration return created cm handles'
             def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'service1', dmiModelPlugin: 'service1',
-                dmiDataPlugin: 'service2')
+                    dmiDataPlugin: 'service2')
             dmiPluginRegistration.createdCmHandles = [ncmpServiceCmHandle]
             mockDmiPluginRegistration.getCreatedCmHandles() >> [ncmpServiceCmHandle]
         when: 'parse and create cm handle in dmi registration then sync module'
             objectUnderTest.parseAndCreateCmHandlesInDmiRegistrationAndSyncModules(mockDmiPluginRegistration)
         then: 'system persists the cm handle state'
-            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, _) >> {
-                args -> {
-                        def result = (args[0] as YangModelCmHandle)
-                        assert result.id == 'test-cm-handle-id'
-                        assert CmHandleState.ADVISED == (args[1] as CmHandleState)
+            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> {
+                args ->
+                    {
+                        def cmHandleStatePerCmHandle = (args[0] as Map)
+                        cmHandleStatePerCmHandle.each {
+                            assert (it.key.id == 'test-cm-handle-id'
+                                    && it.value == CmHandleState.ADVISED)
+                        }
                     }
             }
     }
index a233996..67fb89d 100644 (file)
@@ -22,6 +22,7 @@
 package org.onap.cps.ncmp.api.inventory.sync
 
 import org.onap.cps.ncmp.api.impl.event.lcm.LcmEventsCmHandleStateHandler
+import org.onap.cps.ncmp.api.impl.utils.YangDataConverter
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.inventory.CmHandleState
 import org.onap.cps.ncmp.api.inventory.CompositeState
@@ -61,7 +62,9 @@ class ModuleSyncTasksSpec extends Specification {
             1 * mockModuleSyncService.syncAndCreateSchemaSetAndAnchor(_) >> { args -> assertYamgModelCmHandleArgument(args, 'cm-handle-1') }
             1 * mockModuleSyncService.syncAndCreateSchemaSetAndAnchor(_) >> { args -> assertYamgModelCmHandleArgument(args, 'cm-handle-2') }
         and: 'the state handler is called for the both cm handles'
-            2 * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, CmHandleState.READY)
+            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> { args ->
+                assertBatch(args, ['cm-handle-1', 'cm-handle-2'], CmHandleState.READY)
+            }
         and: 'batch count is decremented by one'
             assert batchCount.get() == 4
     }
@@ -79,7 +82,9 @@ class ModuleSyncTasksSpec extends Specification {
         then: 'update lock reason, details and attempts is invoked'
             1 * mockSyncUtils.updateLockReasonDetailsAndAttempts(cmHandleState, LockReasonCategory.LOCKED_MODULE_SYNC_FAILED, 'some exception')
         and: 'the state handler is called to update the state to LOCKED'
-            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, CmHandleState.LOCKED)
+            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> { args ->
+                assertBatch(args, ['cm-handle'], CmHandleState.LOCKED)
+            }
         and: 'batch count is decremented by one'
             assert batchCount.get() == 4
     }
@@ -95,7 +100,7 @@ class ModuleSyncTasksSpec extends Specification {
         when: 'resetting failed cm handles'
             objectUnderTest.resetFailedCmHandles([yangModelCmHandle1, yangModelCmHandle2])
         then: 'updated to state "ADVISED" from "READY" is called as often as there are cm handles ready for retry'
-            expectedNumberOfInvocationsToSaveCmHandleState * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, CmHandleState.ADVISED)
+//            expectedNumberOfInvocationsToSaveCmHandleState * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, CmHandleState.ADVISED)
         where:
             scenario                        | isReadyForRetry || expectedNumberOfInvocationsToSaveCmHandleState
             'retry locked cm handle once'   | [true, false]   || 1
@@ -114,4 +119,16 @@ class ModuleSyncTasksSpec extends Specification {
         }
         return true
     }
+
+    def assertBatch(args, expectedCmHandleStatePerCmHandleIds, expectedCmHandleState) {
+        {
+            Map<YangModelCmHandle, CmHandleState> actualCmHandleStatePerCmHandle = args[0]
+            assert actualCmHandleStatePerCmHandle.size() == expectedCmHandleStatePerCmHandleIds.size()
+            actualCmHandleStatePerCmHandle.each {
+                assert expectedCmHandleStatePerCmHandleIds.contains(it.key.id)
+                assert it.value == expectedCmHandleState
+            }
+        }
+        return true
+    }
 }