summaryrefslogtreecommitdiffstats
path: root/cps-ri/src
diff options
context:
space:
mode:
authordanielhanrahan <daniel.hanrahan@est.tech>2023-02-21 22:55:05 +0000
committerDaniel Hanrahan <daniel.hanrahan@est.tech>2023-02-24 11:19:05 +0000
commitafa131e65975c46c26af78e5a2e3cdcc652e8306 (patch)
treec331629aeb0590d6ba983a92630a0c6b43c2353b /cps-ri/src
parent003de55a8e6c53643032731e68edc43c0698fd81 (diff)
Improve performance for update data nodes
- Extract method getFragmentEntities from getDataNodes - Remove unused code from getFragmentEntity - Use bulk getFragmentEntities in updateDataNodesAndDescendants - Update performance test timings Issue-ID: CPS-1504 Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech> Change-Id: Icd2c6a0e6009e152de43090cbc23a21349703da2
Diffstat (limited to 'cps-ri/src')
-rw-r--r--cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java73
-rw-r--r--cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy46
-rw-r--r--cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy5
3 files changed, 72 insertions, 52 deletions
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
index 2fba30ee24..dd2d3652ea 100644
--- a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
@@ -120,7 +120,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
private void addNewChildDataNode(final String dataspaceName, final String anchorName,
final String parentNodeXpath, final DataNode newChild) {
final FragmentEntity parentFragmentEntity =
- getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath);
+ getFragmentEntity(dataspaceName, anchorName, parentNodeXpath);
final FragmentEntity newChildAsFragmentEntity =
convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(),
parentFragmentEntity.getAnchor(), newChild);
@@ -136,7 +136,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
private void addChildrenDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath,
final Collection<DataNode> newChildren) {
final FragmentEntity parentFragmentEntity =
- getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath);
+ getFragmentEntity(dataspaceName, anchorName, parentNodeXpath);
final List<FragmentEntity> fragmentEntities = new ArrayList<>(newChildren.size());
try {
newChildren.forEach(newChildAsDataNode -> {
@@ -265,6 +265,12 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
public Collection<DataNode> getDataNodesForMultipleXpaths(final String dataspaceName, final String anchorName,
final Collection<String> xpaths,
final FetchDescendantsOption fetchDescendantsOption) {
+ final Collection<FragmentEntity> fragmentEntities = getFragmentEntities(dataspaceName, anchorName, xpaths);
+ return toDataNodes(fragmentEntities, fetchDescendantsOption);
+ }
+
+ private Collection<FragmentEntity> getFragmentEntities(final String dataspaceName, final String anchorName,
+ final Collection<String> xpaths) {
final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName);
@@ -276,7 +282,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
try {
normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath));
} catch (final PathParsingException e) {
- log.warn("Error parsing xpath \"{}\" in getDataNodesForMultipleXpaths: {}", xpath, e.getMessage());
+ log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage());
}
}
final Collection<FragmentEntity> fragmentEntities =
@@ -288,17 +294,10 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
fragmentEntities.addAll(FragmentEntityArranger.toFragmentEntityTrees(anchorEntity, fragmentExtracts));
}
- return toDataNodes(fragmentEntities, fetchDescendantsOption);
+ return fragmentEntities;
}
- private FragmentEntity getFragmentWithoutDescendantsByXpath(final String dataspaceName,
- final String anchorName,
- final String xpath) {
- return getFragmentByXpath(dataspaceName, anchorName, xpath, FetchDescendantsOption.OMIT_DESCENDANTS);
- }
-
- private FragmentEntity getFragmentByXpath(final String dataspaceName, final String anchorName,
- final String xpath, final FetchDescendantsOption fetchDescendantsOption) {
+ private FragmentEntity getFragmentEntity(final String dataspaceName, final String anchorName, final String xpath) {
final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName);
final FragmentEntity fragmentEntity;
@@ -309,13 +308,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
.stream().findFirst().orElse(null);
} else {
final String normalizedXpath = getNormalizedXpath(xpath);
- if (FetchDescendantsOption.OMIT_DESCENDANTS.equals(fetchDescendantsOption)) {
- fragmentEntity =
- fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath);
- } else {
- fragmentEntity = buildFragmentEntitiesFromFragmentExtracts(anchorEntity, normalizedXpath)
- .stream().findFirst().orElse(null);
- }
+ fragmentEntity =
+ fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath);
}
if (fragmentEntity == null) {
throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath);
@@ -491,7 +485,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
@Override
public void updateDataLeaves(final String dataspaceName, final String anchorName, final String xpath,
final Map<String, Serializable> updateLeaves) {
- final FragmentEntity fragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, xpath);
+ final FragmentEntity fragmentEntity = getFragmentEntity(dataspaceName, anchorName, xpath);
final String currentLeavesAsString = fragmentEntity.getAttributes();
final String mergedLeaves = mergeLeaves(updateLeaves, currentLeavesAsString);
fragmentEntity.setAttributes(mergedLeaves);
@@ -501,8 +495,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
@Override
public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName,
final DataNode dataNode) {
- final FragmentEntity fragmentEntity =
- getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, dataNode.getXpath());
+ final FragmentEntity fragmentEntity = getFragmentEntity(dataspaceName, anchorName, dataNode.getXpath());
updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode);
try {
fragmentRepository.save(fragmentEntity);
@@ -514,21 +507,24 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
}
@Override
- public void updateDataNodesAndDescendants(final String dataspaceName,
- final String anchorName,
- final List<DataNode> dataNodes) {
-
- final Map<DataNode, FragmentEntity> dataNodeFragmentEntityMap = dataNodes.stream()
- .collect(Collectors.toMap(
- dataNode -> dataNode,
- dataNode ->
- getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, dataNode.getXpath())));
- dataNodeFragmentEntityMap.forEach(
- (dataNode, fragmentEntity) -> updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode));
+ public void updateDataNodesAndDescendants(final String dataspaceName, final String anchorName,
+ final List<DataNode> updatedDataNodes) {
+ final Map<String, DataNode> xpathToUpdatedDataNode = updatedDataNodes.stream()
+ .collect(Collectors.toMap(DataNode::getXpath, dataNode -> dataNode));
+
+ final Collection<String> xpaths = xpathToUpdatedDataNode.keySet();
+ final Collection<FragmentEntity> existingFragmentEntities =
+ getFragmentEntities(dataspaceName, anchorName, xpaths);
+
+ for (final FragmentEntity existingFragmentEntity : existingFragmentEntities) {
+ final DataNode updatedDataNode = xpathToUpdatedDataNode.get(existingFragmentEntity.getXpath());
+ updateFragmentEntityAndDescendantsWithDataNode(existingFragmentEntity, updatedDataNode);
+ }
+
try {
- fragmentRepository.saveAll(dataNodeFragmentEntityMap.values());
+ fragmentRepository.saveAll(existingFragmentEntities);
} catch (final StaleStateException staleStateException) {
- retryUpdateDataNodesIndividually(dataspaceName, anchorName, dataNodeFragmentEntityMap.values());
+ retryUpdateDataNodesIndividually(dataspaceName, anchorName, existingFragmentEntities);
}
}
@@ -582,8 +578,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
@Transactional
public void replaceListContent(final String dataspaceName, final String anchorName, final String parentNodeXpath,
final Collection<DataNode> newListElements) {
- final FragmentEntity parentEntity =
- getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath);
+ final FragmentEntity parentEntity = getFragmentEntity(dataspaceName, anchorName, parentNodeXpath);
final String listElementXpathPrefix = getListElementXpathPrefix(newListElements);
final Map<String, FragmentEntity> existingListElementFragmentEntitiesByXPath =
extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listElementXpathPrefix);
@@ -631,7 +626,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
try {
normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath));
} catch (final PathParsingException e) {
- log.debug("Error parsing xpath \"{}\" in deleteDataNodes: {}", xpath, e.getMessage());
+ log.debug("Error parsing xpath \"{}\": {}", xpath, e.getMessage());
}
}
@@ -666,7 +661,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
} else {
parentNodeXpath = CpsPathUtil.getNormalizedParentXpath(targetXpath);
}
- parentFragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath);
+ parentFragmentEntity = getFragmentEntity(dataspaceName, anchorName, parentNodeXpath);
if (CpsPathUtil.isPathToListElement(targetXpath)) {
targetDeleted = deleteDataNode(parentFragmentEntity, targetXpath);
} else {
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
index ac66e8c023..ba42a083ea 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
@@ -88,15 +88,17 @@ class CpsDataPersistenceServiceSpec extends Specification {
}
def 'Handling of StaleStateException (caused by concurrent updates) during update data nodes and descendants.'() {
- given: 'the system contains and can update one datanode'
- def dataNode1 = createDataNodeAndMockRepositoryMethodSupportingIt('/node1', 'OK')
- and: 'the system contains two more datanodes that throw an exception while updating'
- def dataNode2 = createDataNodeAndMockRepositoryMethodSupportingIt('/node2', 'EXCEPTION')
- def dataNode3 = createDataNodeAndMockRepositoryMethodSupportingIt('/node3', 'EXCEPTION')
+ given: 'the system can update one datanode and has two more datanodes that throw an exception while updating'
+ def dataNodes = createDataNodesAndMockRepositoryMethodSupportingThem([
+ '/node1': 'OK',
+ '/node2': 'EXCEPTION',
+ '/node3': 'EXCEPTION'])
+ and: 'db contains an anchor'
+ mockAnchorRepository.getByDataspaceAndName(*_) >> new AnchorEntity(id:123)
and: 'the batch update will therefore also fail'
mockFragmentRepository.saveAll(*_) >> { throw new StaleStateException("concurrent updates") }
when: 'attempt batch update data nodes'
- objectUnderTest.updateDataNodesAndDescendants('some-dataspace', 'some-anchor', [dataNode1, dataNode2, dataNode3])
+ objectUnderTest.updateDataNodesAndDescendants('some-dataspace', 'some-anchor', dataNodes)
then: 'concurrency exception is thrown'
def thrown = thrown(ConcurrencyException)
assert thrown.message == 'Concurrent Transactions'
@@ -199,7 +201,9 @@ class CpsDataPersistenceServiceSpec extends Specification {
def 'update data node and descendants: #scenario'(){
given: 'mocked responses'
- mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath') >> new FragmentEntity(xpath: '/test/xpath', childFragments: [])
+ mockAnchorRepository.getByDataspaceAndName(_, _) >> new AnchorEntity(id:123)
+ mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, [] as Set) >> []
+ mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, ['/test/xpath'] as Set) >> [new FragmentEntity(xpath: '/test/xpath', childFragments: [])]
when: 'replace data node tree'
objectUnderTest.updateDataNodesAndDescendants('dataspaceName', 'anchorName', dataNodes)
then: 'call fragment repository save all method'
@@ -211,9 +215,12 @@ class CpsDataPersistenceServiceSpec extends Specification {
}
def 'update data nodes and descendants'() {
- given: 'the fragment repository returns a fragment entity related to the xpath input'
- mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath1') >> new FragmentEntity(xpath: '/test/xpath1', childFragments: [])
- mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath2') >> new FragmentEntity(xpath: '/test/xpath2', childFragments: [])
+ given: 'the fragment repository returns fragment entities related to the xpath inputs'
+ mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, ['/test/xpath1', '/test/xpath2'] as Set) >> [
+ new FragmentEntity(xpath: '/test/xpath1', childFragments: []),
+ new FragmentEntity(xpath: '/test/xpath2', childFragments: [])]
+ and: 'db contains an anchor'
+ mockAnchorRepository.getByDataspaceAndName(*_) >> new AnchorEntity(id:123)
and: 'some data nodes with descendants'
def dataNode1 = new DataNode(xpath: '/test/xpath1', leaves: ['id': 'testId1'], childDataNodes: [new DataNode(xpath: '/test/xpath1/child', leaves: ['id': 'childTestId1'])])
def dataNode2 = new DataNode(xpath: '/test/xpath2', leaves: ['id': 'testId2'], childDataNodes: [new DataNode(xpath: '/test/xpath2/child', leaves: ['id': 'childTestId2'])])
@@ -239,6 +246,25 @@ class CpsDataPersistenceServiceSpec extends Specification {
return dataNode
}
+ def createDataNodesAndMockRepositoryMethodSupportingThem(Map<String, String> xpathToScenarioMap) {
+ def dataNodes = []
+ def fragmentEntities = []
+ xpathToScenarioMap.each {
+ def xpath = it.key
+ def scenario = it.value
+ def dataNode = new DataNodeBuilder().withXpath(xpath).build()
+ dataNodes.add(dataNode)
+ def fragmentEntity = new FragmentEntity(xpath: xpath, childFragments: [])
+ fragmentEntities.add(fragmentEntity)
+ mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, xpath) >> fragmentEntity
+ if ('EXCEPTION' == scenario) {
+ mockFragmentRepository.save(fragmentEntity) >> { throw new StaleStateException("concurrent updates") }
+ }
+ }
+ mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, xpathToScenarioMap.keySet()) >> fragmentEntities
+ return dataNodes
+ }
+
def mockFragmentWithJson(json) {
def anchorEntity = new AnchorEntity(id:123)
mockAnchorRepository.getByDataspaceAndName(*_) >> anchorEntity
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy
index cfd1093a99..3562419c05 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy
@@ -108,7 +108,6 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
stopWatch.stop()
def readDurationInMillis = stopWatch.getTotalTimeMillis()
then: 'read duration is under #allowedDuration milliseconds'
- assert readDurationInMillis < allowedDuration
recordAndAssertPerformance("Query many descendants by cpspath (${scenario})", allowedDuration, readDurationInMillis)
and: 'data node is returned with all the descendants populated'
assert result.size() == NUMBER_OF_CHILDREN
@@ -153,7 +152,7 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
objectUnderTest.updateDataNodesAndDescendants(PERF_DATASPACE, PERF_ANCHOR, dataNodes)
stopWatch.stop()
def updateDurationInMillis = stopWatch.getTotalTimeMillis()
- then: 'update duration is under 1400 milliseconds'
- recordAndAssertPerformance('Update data nodes without descendants', 1400, updateDurationInMillis)
+ then: 'update duration is under 600 milliseconds'
+ recordAndAssertPerformance('Update data nodes without descendants', 600, updateDurationInMillis)
}
}