From 17fbc4851079ccd22be338c28cdf5c0637d94944 Mon Sep 17 00:00:00 2001 From: Felix Haller Date: Mon, 26 Jul 2021 08:18:44 +0200 Subject: [PATCH] fix revocation list insert and delete batch statements --- .../verifier/data/RevokedCertDataService.java | 3 +- .../impl/JdbcRevokedCertDataServiceImpl.java | 56 ++++++++++++++----- .../ws/client/RevocationListSyncer.java | 16 +++--- .../verifier/ws/config/SchedulingConfig.java | 4 +- .../ws/controller/dev/DevController.java | 2 +- ch-covidcertificate-backend-verifier/pom.xml | 5 ++ 6 files changed, 61 insertions(+), 25 deletions(-) diff --git a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/RevokedCertDataService.java b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/RevokedCertDataService.java index 64ab5eb4..05bce319 100644 --- a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/RevokedCertDataService.java +++ b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/RevokedCertDataService.java @@ -13,11 +13,12 @@ import ch.admin.bag.covidcertificate.backend.verifier.model.DbRevokedCert; import ch.admin.bag.covidcertificate.backend.verifier.model.cert.db.RevokedCertsUpdateResponse; import java.util.List; +import java.util.Set; public interface RevokedCertDataService { /** upserts the given revoked uvcis into the db */ - public RevokedCertsUpdateResponse replaceRevokedCerts(List revokedUvcis); + public RevokedCertsUpdateResponse replaceRevokedCerts(Set revokedUvcis); /** returns the next batch of revoked certs after `since` */ public List findRevokedCerts(Long since); diff --git a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/impl/JdbcRevokedCertDataServiceImpl.java b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/impl/JdbcRevokedCertDataServiceImpl.java index b38aa4db..dfdfda93 100644 --- a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/impl/JdbcRevokedCertDataServiceImpl.java +++ b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-data/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/data/impl/JdbcRevokedCertDataServiceImpl.java @@ -14,9 +14,12 @@ import ch.admin.bag.covidcertificate.backend.verifier.data.mapper.RevokedCertRowMapper; import ch.admin.bag.covidcertificate.backend.verifier.model.DbRevokedCert; import ch.admin.bag.covidcertificate.backend.verifier.model.cert.db.RevokedCertsUpdateResponse; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import javax.sql.DataSource; +import org.apache.commons.collections4.ListUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.dao.EmptyResultDataAccessException; @@ -45,30 +48,40 @@ public JdbcRevokedCertDataServiceImpl(DataSource dataSource, int revokedCertBatc @Transactional(readOnly = false) @Override - public RevokedCertsUpdateResponse replaceRevokedCerts(List revokedUvcis) { - int insertCount = insertNewRevokedCerts(revokedUvcis); - int removeCount = removeRevokedCertsNotIn(revokedUvcis); + public RevokedCertsUpdateResponse replaceRevokedCerts(Set revokedUvcis) { + Set existingUvcis = findAllRevokedUvcis(); + int insertCount = insertNewRevokedCerts(revokedUvcis, existingUvcis); + int removeCount = removeRevokedCertsNotIn(revokedUvcis, existingUvcis); return new RevokedCertsUpdateResponse(insertCount, removeCount); } - private int insertNewRevokedCerts(List revokedUvcis) { + private int insertNewRevokedCerts(Set revokedUvcis, Set existingUvcis) { if (revokedUvcis != null && !revokedUvcis.isEmpty()) { - List existingUvcis = - jt.queryForList( - "select uvci from t_revoked_cert", - new MapSqlParameterSource(), - String.class); List toInsert = revokedUvcis.stream() .filter(uvci -> !existingUvcis.contains(uvci)) .collect(Collectors.toList()); - revokedCertInsert.executeBatch(createParams(toInsert)); + + // insert in batches + final int maxBatchSize = 10000; + for (List batchToInsert : ListUtils.partition(toInsert, maxBatchSize)) { + revokedCertInsert.executeBatch(createParams(batchToInsert)); + } + return toInsert.size(); } else { return 0; } } + private Set findAllRevokedUvcis() { + return new HashSet<>( + jt.queryForList( + "select distinct uvci from t_revoked_cert", + new MapSqlParameterSource(), + String.class)); + } + private MapSqlParameterSource[] createParams(List revokedUvcis) { if (revokedUvcis == null) { return null; @@ -82,12 +95,27 @@ private MapSqlParameterSource[] createParams(List revokedUvcis) { return params; } - private int removeRevokedCertsNotIn(List revokedUvcis) { + private int removeRevokedCertsNotIn(Set uvcisToKeep, Set existingUvcis) { String sql = "delete from t_revoked_cert"; - if (revokedUvcis != null && !revokedUvcis.isEmpty()) { - sql += " where uvci not in (:to_keep)"; + if (uvcisToKeep != null && !uvcisToKeep.isEmpty()) { + sql += " where uvci in (:to_delete)"; + + List toDelete = + existingUvcis.stream() + .filter(uvci -> !uvcisToKeep.contains(uvci)) + .collect(Collectors.toList()); + + // delete in batches + int deleteCount = 0; + final int maxBatchSize = 10000; + for (List batchToDelete : ListUtils.partition(toDelete, maxBatchSize)) { + deleteCount += + jt.update(sql, new MapSqlParameterSource("to_delete", batchToDelete)); + } + return deleteCount; + } else { + return jt.update(sql, new MapSqlParameterSource()); } - return jt.update(sql, new MapSqlParameterSource("to_keep", revokedUvcis)); } @Transactional(readOnly = true) diff --git a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/client/RevocationListSyncer.java b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/client/RevocationListSyncer.java index f64dc653..6298bb33 100644 --- a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/client/RevocationListSyncer.java +++ b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/client/RevocationListSyncer.java @@ -12,9 +12,9 @@ import ch.admin.bag.covidcertificate.backend.verifier.data.RevokedCertDataService; import ch.admin.bag.covidcertificate.backend.verifier.model.cert.db.RevokedCertsUpdateResponse; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; +import java.util.HashSet; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -42,28 +42,30 @@ public void updateRevokedCerts() { logger.info("updating revoked certs"); try { - List revokedCerts = downloadRevokedCerts(); + long start = System.currentTimeMillis(); + Set revokedCerts = downloadRevokedCerts(); logger.info("downloaded {} revoked certs", revokedCerts.size()); RevokedCertsUpdateResponse updateResponse = revokedCertDataService.replaceRevokedCerts(revokedCerts); logger.info( - "finished updating revoked certs. inserted {}, removed {}", + "finished updating revoked certs. inserted {}, removed {}. took {}ms", updateResponse.getInsertCount(), - updateResponse.getRemoveCount()); + updateResponse.getRemoveCount(), + System.currentTimeMillis() - start); } catch (Exception e) { logger.error("revoked certs update failed", e); } } - private List downloadRevokedCerts() { + private Set downloadRevokedCerts() { final var requestEndpoint = baseurl + endpoint; final var uri = UriComponentsBuilder.fromHttpUrl(requestEndpoint).build().toUri(); final RequestEntity requestEntity = RequestEntity.get(uri).headers(createDownloadHeaders()).build(); final var response = rt.exchange(requestEntity, String[].class).getBody(); - return new ArrayList<>(Arrays.asList(response)); + return new HashSet<>(Arrays.asList(response)); } private HttpHeaders createDownloadHeaders() { diff --git a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/config/SchedulingConfig.java b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/config/SchedulingConfig.java index 9b580cdc..1a857e52 100644 --- a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/config/SchedulingConfig.java +++ b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/config/SchedulingConfig.java @@ -41,8 +41,8 @@ public void syncRevocationListOnStartup() { revocationListSyncer.updateRevokedCerts(); } - // Sync revocation list every full hour (default) - @Scheduled(cron = "${revocationList.sync.cron:0 0 * ? * *}") + // Sync revocation list every 5 minutes from the full hour (default) + @Scheduled(cron = "${revocationList.sync.cron:0 0/5 * ? * *}") @SchedulerLock(name = "revocation_list_sync", lockAtLeastFor = "PT15S") public void syncRevocationList() { LockAssert.assertLocked(); diff --git a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/dev/DevController.java b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/dev/DevController.java index 9fba4ec6..9fdead2d 100644 --- a/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/dev/DevController.java +++ b/ch-covidcertificate-backend-verifier/ch-covidcertificate-backend-verifier-ws/src/main/java/ch/admin/bag/covidcertificate/backend/verifier/ws/controller/dev/DevController.java @@ -27,7 +27,7 @@ public class DevController { @GetMapping(value = "/v1/revocation-list") public @ResponseBody ResponseEntity> getMockRevokedCerts() { List response = new ArrayList<>(); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < 100010; i++) { response.add("urn:uvci:01:CH:MOCK" + i); } return ResponseEntity.ok(response); diff --git a/ch-covidcertificate-backend-verifier/pom.xml b/ch-covidcertificate-backend-verifier/pom.xml index 15ba316a..919023f9 100644 --- a/ch-covidcertificate-backend-verifier/pom.xml +++ b/ch-covidcertificate-backend-verifier/pom.xml @@ -71,6 +71,11 @@ org.apache.commons commons-lang3 + + org.apache.commons + commons-collections4 + 4.4 + org.apache.httpcomponents httpclient