From: sourabh_sourabh Date: Fri, 2 Sep 2022 08:40:35 +0000 (+0100) Subject: Performance Improvement: Use save batches of cmhandles X-Git-Tag: 3.1.0~13 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=cps.git;a=commitdiff_plain;h=5c1c7a8f467c0c7e673aa81de9e39766c67eb20f Performance Improvement: Use save batches of cmhandles -Used cm handle batch to persist from state handler. Issue-ID: CPS-1230 Signed-off-by: sourabh_sourabh Change-Id: I68b7fde7dc85818b818f1af588344c26b549d87b --- diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java index c21c74bfb..e8a64115a 100755 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java @@ -300,14 +300,18 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService public List parseAndCreateCmHandlesInDmiRegistrationAndSyncModules( final DmiPluginRegistration dmiPluginRegistration) { List cmHandleRegistrationResponses = new ArrayList<>(); + final Map 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 registerNewCmHandles(final Map + cmHandleStatePerCmHandle) { + final List 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)); } } } diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasks.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasks.java index 597e2ba8e..ada3dc674 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasks.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasks.java @@ -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 cmHandleStatePerCmHandle) { - // To be refactored as part of CPS-1231; Use state-save-batch capability (depends sub-task12, 13) - for (final Map.Entry entry : cmHandleStatePerCmHandle.entrySet()) { - lcmEventsCmHandleStateHandler.updateCmHandleState(entry.getKey(), entry.getValue()); - } - } - } diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java index 1da2aa943..b7faf09a9 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java @@ -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 createSuccessResponses(final List cmHandleIds) { + return cmHandleIds.stream().map(CmHandleRegistrationResponse::createSuccessResponse) + .collect(Collectors.toList()); + } + public enum Status { SUCCESS, FAILURE; } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy index ed985ec00..86a32a1a5 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy @@ -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' diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy index 02cfb152c..def0db32d 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy @@ -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) + } } } } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy index a2339963e..67fb89dbb 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy @@ -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 actualCmHandleStatePerCmHandle = args[0] + assert actualCmHandleStatePerCmHandle.size() == expectedCmHandleStatePerCmHandleIds.size() + actualCmHandleStatePerCmHandle.each { + assert expectedCmHandleStatePerCmHandleIds.contains(it.key.id) + assert it.value == expectedCmHandleState + } + } + return true + } }