diff options
author | 2023-02-20 14:01:45 +0000 | |
---|---|---|
committer | 2023-02-21 21:23:28 +0000 | |
commit | 5f91567ed17f96716e7a227702eed2ea96bb9e63 (patch) | |
tree | a651475f6e02a53f86e558b2c5661380fb294c2e /cps-ri/src | |
parent | 1450fd0687433278ce61ae56dfcbf42e0edddce3 (diff) |
Improve performance of deleteDataNodes SQL
- Use SQL IN operator instead of temp table when deleting nodes
- Use Postgresql LIKE ANY array operator when deleting lists
- Update delete perf test timings
- Refactor adding cascade delete constraint
Issue-ID: CPS-1502
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: Ic90b867e7c71ec1981f05a9122322ece84dd8bde
Diffstat (limited to 'cps-ri/src')
2 files changed, 46 insertions, 43 deletions
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java index 0e4d359da..5c5458a03 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java @@ -21,8 +21,11 @@ package org.onap.cps.spi.repository; import java.util.Collection; +import java.util.Collections; +import java.util.stream.Collectors; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; +import javax.persistence.Query; import lombok.RequiredArgsConstructor; @RequiredArgsConstructor @@ -40,55 +43,55 @@ public class FragmentNativeRepositoryImpl implements FragmentNativeRepository { @PersistenceContext private final EntityManager entityManager; - private final TempTableCreator tempTableCreator; - @Override public void deleteFragmentEntity(final long fragmentEntityId) { entityManager.createNativeQuery( - DROP_FRAGMENT_CONSTRAINT - + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE - + "DELETE FROM fragment WHERE id = ?;" - + DROP_FRAGMENT_CONSTRAINT - + ADD_ORIGINAL_FRAGMENT_CONSTRAINT) + addFragmentConstraintWithDeleteCascade("DELETE FROM fragment WHERE id = ?")) .setParameter(1, fragmentEntityId) .executeUpdate(); } @Override - // Accept security hotspot as temporary table name in SQL query is created internally, not from user input. - @SuppressWarnings("squid:S2077") public void deleteByAnchorIdAndXpaths(final int anchorId, final Collection<String> xpaths) { - if (!xpaths.isEmpty()) { - final String tempTableName = tempTableCreator.createTemporaryTable("xpathsToDelete", xpaths, "xpath"); - entityManager.createNativeQuery( - DROP_FRAGMENT_CONSTRAINT - + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE - + "DELETE FROM fragment f USING " + tempTableName + " t" - + " WHERE f.anchor_id = :anchorId AND f.xpath = t.xpath;" - + DROP_FRAGMENT_CONSTRAINT - + ADD_ORIGINAL_FRAGMENT_CONSTRAINT) - .setParameter("anchorId", anchorId) - .executeUpdate(); - } + final String queryString = addFragmentConstraintWithDeleteCascade( + "DELETE FROM fragment f WHERE f.anchor_id = ? AND (f.xpath IN (:parameterPlaceholders))"); + executeUpdateWithAnchorIdAndCollection(queryString, anchorId, xpaths); } @Override - // Accept security hotspot as temporary table name in SQL query is created internally, not from user input. + public void deleteListsByAnchorIdAndXpaths(final int anchorId, final Collection<String> listXpaths) { + final Collection<String> listXpathPatterns = + listXpaths.stream().map(listXpath -> listXpath + "[%").collect(Collectors.toSet()); + final String queryString = addFragmentConstraintWithDeleteCascade( + "DELETE FROM fragment f WHERE f.anchor_id = ? AND (f.xpath LIKE ANY (array[:parameterPlaceholders]))"); + executeUpdateWithAnchorIdAndCollection(queryString, anchorId, listXpathPatterns); + } + + // Accept security hotspot as placeholders in SQL query are created internally, not from user input. @SuppressWarnings("squid:S2077") - public void deleteListsByAnchorIdAndXpaths(final int anchorId, final Collection<String> xpaths) { - if (!xpaths.isEmpty()) { - final String tempTableName = tempTableCreator.createTemporaryTable("xpathsToDelete", xpaths, "xpath"); - entityManager.createNativeQuery( - DROP_FRAGMENT_CONSTRAINT - + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE - + "DELETE FROM fragment f USING " + tempTableName + " t" - + " WHERE f.anchor_id = :anchorId AND f.xpath LIKE CONCAT(t.xpath, :xpathListPattern);" - + DROP_FRAGMENT_CONSTRAINT - + ADD_ORIGINAL_FRAGMENT_CONSTRAINT) - .setParameter("anchorId", anchorId) - .setParameter("xpathListPattern", "[%%") - .executeUpdate(); + private void executeUpdateWithAnchorIdAndCollection(final String sqlTemplate, final int anchorId, + final Collection<String> collection) { + if (!collection.isEmpty()) { + final String parameterPlaceholders = String.join(",", Collections.nCopies(collection.size(), "?")); + final String queryStringWithParameterPlaceholders = + sqlTemplate.replaceFirst(":parameterPlaceholders\\b", parameterPlaceholders); + + final Query query = entityManager.createNativeQuery(queryStringWithParameterPlaceholders); + query.setParameter(1, anchorId); + int parameterIndex = 2; + for (final String parameterValue : collection) { + query.setParameter(parameterIndex++, parameterValue); + } + query.executeUpdate(); } } + private static String addFragmentConstraintWithDeleteCascade(final String queryString) { + return DROP_FRAGMENT_CONSTRAINT + + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE + + queryString + ";" + + DROP_FRAGMENT_CONSTRAINT + + ADD_ORIGINAL_FRAGMENT_CONSTRAINT; + } + } diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy index db09382ab..8e74b6222 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy @@ -67,8 +67,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Batch delete 100 children', 300, deleteDurationInMillis) + then: 'delete duration is under 250 milliseconds' + recordAndAssertPerformance('Batch delete 100 children', 250, deleteDurationInMillis) } def 'Delete 50 grandchildren (that have no descendants)'() { @@ -97,8 +97,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Batch delete 500 grandchildren', 300, deleteDurationInMillis) + then: 'delete duration is under 125 milliseconds' + recordAndAssertPerformance('Batch delete 500 grandchildren', 125, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -135,8 +135,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 350 milliseconds' - recordAndAssertPerformance('Batch delete 100 whole lists', 350, deleteDurationInMillis) + then: 'delete duration is under 250 milliseconds' + recordAndAssertPerformance('Batch delete 100 whole lists', 250, deleteDurationInMillis) } def 'Delete 10 list elements'() { @@ -165,8 +165,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Batch delete 500 lists elements', 300, deleteDurationInMillis) + then: 'delete duration is under 125 milliseconds' + recordAndAssertPerformance('Batch delete 500 lists elements', 125, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) |