From a0aa860665b94ce35ecd6253e69959b7cf08ddb7 Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Wed, 1 Feb 2023 14:48:52 +0000 Subject: [PATCH] CmHandle batch deletion - Use plural deleteDataNodes to remove CmHandles in batches, falling back to individual delete on error - Use single deleteDataNode instead of deleteListOrListElement for individual delete Issue-ID: CPS-1464 Signed-off-by: danielhanrahan Change-Id: If09f22478df8703290c8fc24aa6fe2a11c90788a --- .../api/impl/NetworkCmProxyDataServiceImpl.java | 68 +++++++++++++++------- .../ncmp/api/inventory/InventoryPersistence.java | 11 +++- .../api/inventory/InventoryPersistenceImpl.java | 8 ++- ...rkCmProxyDataServiceImplRegistrationSpec.groovy | 13 +++-- .../inventory/InventoryPersistenceImplSpec.groovy | 8 +++ 5 files changed, 79 insertions(+), 29 deletions(-) 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 8b80a0341..a5bd60605 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 @@ -1,7 +1,7 @@ /* * ============LICENSE_START======================================================= * Copyright (C) 2021 highstreet technologies GmbH - * Modifications Copyright (C) 2021-2022 Nordix Foundation + * Modifications Copyright (C) 2021-2023 Nordix Foundation * Modifications Copyright (C) 2021 Pantheon.tech * Modifications Copyright (C) 2021-2022 Bell Canada * ================================================================================ @@ -27,6 +27,7 @@ import static org.onap.cps.ncmp.api.impl.constants.DmiRegistryConstants.NFP_OPER import static org.onap.cps.ncmp.api.impl.operations.DmiRequestBody.OperationEnum; import static org.onap.cps.ncmp.api.impl.utils.RestQueryParametersValidator.validateCmHandleQueryParameters; +import com.google.common.collect.Lists; import com.hazelcast.map.IMap; import java.time.OffsetDateTime; import java.util.ArrayList; @@ -78,6 +79,7 @@ import org.springframework.stereotype.Service; @RequiredArgsConstructor public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService { + private static final int DELETE_BATCH_SIZE = 100; private final JsonObjectMapper jsonObjectMapper; private final DmiDataOperations dmiDataOperations; private final NetworkCmProxyDataServicePropertyHandler networkCmProxyDataServicePropertyHandler; @@ -330,25 +332,19 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService .collect(Collectors.toList()); updateCmHandleStateBatch(yangModelCmHandles, CmHandleState.DELETING); - for (final String cmHandleId : tobeRemovedCmHandles) { + for (final List tobeRemovedCmHandleBatch : Lists.partition(tobeRemovedCmHandles, DELETE_BATCH_SIZE)) { try { - deleteCmHandleFromDbAndModuleSyncMap(cmHandleId); - cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createSuccessResponse(cmHandleId)); - } catch (final DataNodeNotFoundException dataNodeNotFoundException) { - log.error("Unable to find dataNode for cmHandleId : {} , caused by : {}", - cmHandleId, dataNodeNotFoundException.getMessage()); - cmHandleRegistrationResponses.add(CmHandleRegistrationResponse - .createFailureResponse(cmHandleId, RegistrationError.CM_HANDLE_DOES_NOT_EXIST)); - } catch (final DataValidationException dataValidationException) { - log.error("Unable to de-register cm-handle id: {}, caused by: {}", - cmHandleId, dataValidationException.getMessage()); - cmHandleRegistrationResponses.add(CmHandleRegistrationResponse - .createFailureResponse(cmHandleId, RegistrationError.CM_HANDLE_INVALID_ID)); - } catch (final Exception exception) { - log.error("Unable to de-register cm-handle id : {} , caused by : {}", - cmHandleId, exception.getMessage()); - cmHandleRegistrationResponses.add( - CmHandleRegistrationResponse.createFailureResponse(cmHandleId, exception)); + batchDeleteCmHandlesFromDbAndModuleSyncMap(tobeRemovedCmHandleBatch); + tobeRemovedCmHandleBatch.forEach(cmHandleId -> + cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createSuccessResponse(cmHandleId))); + + } catch (final Exception batchException) { + log.error("Unable to de-register cm-handle batch, retrying on each cm handle"); + for (final String cmHandleId : tobeRemovedCmHandleBatch) { + final CmHandleRegistrationResponse cmHandleRegistrationResponse = + deleteCmHandleAndGetCmHandleRegistrationResponse(cmHandleId); + cmHandleRegistrationResponses.add(cmHandleRegistrationResponse); + } } } @@ -356,6 +352,26 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService return cmHandleRegistrationResponses; } + private CmHandleRegistrationResponse deleteCmHandleAndGetCmHandleRegistrationResponse(final String cmHandleId) { + try { + deleteCmHandleFromDbAndModuleSyncMap(cmHandleId); + return CmHandleRegistrationResponse.createSuccessResponse(cmHandleId); + } catch (final DataNodeNotFoundException dataNodeNotFoundException) { + log.error("Unable to find dataNode for cmHandleId : {} , caused by : {}", + cmHandleId, dataNodeNotFoundException.getMessage()); + return CmHandleRegistrationResponse.createFailureResponse(cmHandleId, + RegistrationError.CM_HANDLE_DOES_NOT_EXIST); + } catch (final DataValidationException dataValidationException) { + log.error("Unable to de-register cm-handle id: {}, caused by: {}", + cmHandleId, dataValidationException.getMessage()); + return CmHandleRegistrationResponse.createFailureResponse(cmHandleId, + RegistrationError.CM_HANDLE_INVALID_ID); + } catch (final Exception exception) { + log.error("Unable to de-register cm-handle id : {} , caused by : {}", cmHandleId, exception.getMessage()); + return CmHandleRegistrationResponse.createFailureResponse(cmHandleId, exception); + } + } + private void updateCmHandleStateBatch(final List yangModelCmHandles, final CmHandleState cmHandleState) { final Map cmHandleIdsToBeRemoved = new HashMap<>(); @@ -366,10 +382,22 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService private void deleteCmHandleFromDbAndModuleSyncMap(final String cmHandleId) { inventoryPersistence.deleteSchemaSetWithCascade(cmHandleId); - inventoryPersistence.deleteListOrListElement("/dmi-registry/cm-handles[@id='" + cmHandleId + "']"); + inventoryPersistence.deleteDataNode("/dmi-registry/cm-handles[@id='" + cmHandleId + "']"); removeDeletedCmHandleFromModuleSyncMap(cmHandleId); } + private void batchDeleteCmHandlesFromDbAndModuleSyncMap(final Collection tobeRemovedCmHandles) { + tobeRemovedCmHandles.forEach(inventoryPersistence::deleteSchemaSetWithCascade); + inventoryPersistence.deleteDataNodes(mapCmHandleIdsToXpaths(tobeRemovedCmHandles)); + tobeRemovedCmHandles.forEach(this::removeDeletedCmHandleFromModuleSyncMap); + } + + private Collection mapCmHandleIdsToXpaths(final Collection cmHandles) { + return cmHandles.stream() + .map(cmHandleId -> "/dmi-registry/cm-handles[@id='" + cmHandleId + "']") + .collect(Collectors.toSet()); + } + // CPS-1239 Robustness cleaning of in progress cache private void removeDeletedCmHandleFromModuleSyncMap(final String deletedCmHandleId) { if (moduleSyncStartedOnCmHandles.remove(deletedCmHandleId) != null) { diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java index 6d006d9e2..55442ecff 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2022 Nordix Foundation + * Copyright (C) 2022-2023 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -147,9 +147,16 @@ public interface InventoryPersistence { void replaceListContent(String parentNodeXpath, Collection dataNodes); /** - * Deletes data node for given anchor and dataspace. + * Deletes data node. * * @param dataNodeXpath data node xpath */ void deleteDataNode(String dataNodeXpath); + + /** + * Deletes multiple data nodes. + * + * @param dataNodeXpaths data node xpaths + */ + void deleteDataNodes(Collection dataNodeXpaths); } diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java index dd165853d..29712f4d0 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java @@ -184,7 +184,11 @@ public class InventoryPersistenceImpl implements InventoryPersistence { @Override public void deleteDataNode(final String dataNodeXpath) { - cpsDataService.deleteDataNode(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, dataNodeXpath, - NO_TIMESTAMP); + cpsDataService.deleteDataNode(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, dataNodeXpath, NO_TIMESTAMP); + } + + @Override + public void deleteDataNodes(final Collection dataNodeXpaths) { + cpsDataService.deleteDataNodes(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, dataNodeXpaths, NO_TIMESTAMP); } } 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 1ebd69eb6..5824a4729 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 @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2021-2022 Nordix Foundation + * Copyright (C) 2021-2023 Nordix Foundation * Modifications Copyright (C) 2022 Bell Canada * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); @@ -256,7 +256,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification { and: 'method to delete relevant schema set is called once' 1 * mockInventoryPersistence.deleteSchemaSetWithCascade(_) and: 'method to delete relevant list/list element is called once' - 1 * mockInventoryPersistence.deleteListOrListElement(_) + 1 * mockInventoryPersistence.deleteDataNodes(_) and: 'successful response is received' assert response.getRemovedCmHandles().size() == 1 with(response.getRemovedCmHandles().get(0)) { @@ -275,8 +275,10 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification { given: 'a registration with three cm-handles to be deleted' def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server', removedCmHandles: ['cmhandle1', 'cmhandle2', 'cmhandle3']) + and: 'cm-handle deletion fails on batch' + mockInventoryPersistence.deleteDataNodes(_) >> { throw new RuntimeException("Failed") } and: 'cm-handle deletion is successful for 1st and 3rd; failed for 2nd' - mockInventoryPersistence.deleteListOrListElement(_) >> {} >> { throw new RuntimeException("Failed") } >> {} + mockInventoryPersistence.deleteDataNode(_) >> {} >> { throw new RuntimeException("Failed") } >> {} when: 'registration is updated to delete cmhandles' def response = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration) then: 'a response is received for all cm-handles' @@ -315,7 +317,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification { then: 'no exception is thrown' noExceptionThrown() and: 'cm-handle is not deleted' - 0 * mockInventoryPersistence.deleteListOrListElement(_) + 0 * mockInventoryPersistence.deleteDataNodes(_) and: 'the cmHandle state is not updated to "DELETED"' 0 * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, CmHandleState.DELETED) and: 'a failure response is received' @@ -333,7 +335,8 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification { def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server', removedCmHandles: ['cmhandle']) and: 'cm-handle deletion throws exception' - mockInventoryPersistence.deleteListOrListElement(_) >> { throw deleteListElementException } + mockInventoryPersistence.deleteDataNodes(_) >> { throw deleteListElementException } + mockInventoryPersistence.deleteDataNode(_) >> { throw deleteListElementException } when: 'registration is updated to delete cmhandle' 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/inventory/InventoryPersistenceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy index 355487f64..2ca0e9964 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy @@ -270,4 +270,12 @@ class InventoryPersistenceImplSpec extends Specification { 'sample dataNode xpath', NO_TIMESTAMP); } + def 'Delete multiple data nodes via xPath'() { + when: 'Delete data nodes method is called with multiple xpaths as parameters' + objectUnderTest.deleteDataNodes(['xpath1', 'xpath2']) + then: 'the cps data service method to delete data nodes is invoked once with the same xPaths' + 1 * mockCpsDataService.deleteDataNodes('NCMP-Admin', 'ncmp-dmi-registry', + ['xpath1', 'xpath2'], NO_TIMESTAMP); + } + } -- 2.16.6