Commit 7f566b0c authored by Matija Obreza's avatar Matija Obreza

Improve upsert performace

- query only on Accession table using IX_id3
- logging
parent f71bcc45
......@@ -89,8 +89,8 @@ public class ApiExceptionHandler {
@ResponseStatus(code = HttpStatus.UNAUTHORIZED)
@ExceptionHandler({ AccessDeniedException.class })
@ResponseBody
public ApiError<Exception> handleAccessDenied(final Exception e, final WebRequest request) {
LOG.warn("Authentication is required.", e);
public ApiError<Exception> handleAccessDenied(final Exception e, final HttpServletRequest request) {
LOG.warn("Authentication is required {} {}", request.getMethod(), request.getRequestURL());
return new ApiError<>(e);
}
......
......@@ -62,7 +62,7 @@ import io.swagger.annotations.Api;
*/
@RestController("catalogMeApi0")
@RequestMapping(MeController.API_URL)
@PreAuthorize("isAuthenticated()")
@PreAuthorize("isAuthenticated() && hasRole('USER')") // Don't allow OAuth clients here
@Api(tags = { "me" })
public class MeController {
......
......@@ -776,7 +776,7 @@ public class DatasetServiceImpl implements DatasetService {
*/
private Set<AccessionRef> lookupMatchingAccessions(final Set<AccessionRef> accessionRefs) {
List<AccessionRef> list = new ArrayList<AccessionRef>(accessionRefs);
List<Accession> foundAccessions = accessionRepository.findById(list);
List<Accession> foundAccessions = accessionRepository.find(list);
LOG.info("Found {} matches for {} identifiers", foundAccessions.size(), accessionRefs.size());
......
......@@ -26,6 +26,7 @@ import javax.persistence.RollbackException;
import org.apache.commons.lang3.RandomUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.time.StopWatch;
import org.genesys2.server.api.ApiBaseController;
import org.genesys2.server.api.PleaseRetryException;
import org.genesys2.server.api.model.AccessionHeaderJson;
......@@ -207,6 +208,8 @@ public class AccessionController extends ApiBaseController {
@RequestMapping(value = "/{instCode}/upsert", method = { RequestMethod.POST, RequestMethod.PUT }, produces = { MediaType.APPLICATION_JSON_VALUE })
public List<AccessionOpResponse> upsertInstituteAccession(@PathVariable("instCode") String instCode, @RequestBody ArrayNode updates) throws Exception {
StopWatch stopWatch = StopWatch.createStarted();
LOG.trace("Received:\n {}", updates);
upgradeToV2(updates);
......@@ -219,9 +222,15 @@ public class AccessionController extends ApiBaseController {
for (int tryCount = 0; tryCount < UPLOAD_RETRIES; tryCount++) {
try {
return uploader.upsertAccessions(institute, updates);
List<AccessionOpResponse> res = uploader.upsertAccessions(institute, updates);
LOG.info("Processed {} accessions for {} in {} tries in {}ms", updates.size(), instCode, tryCount + 1, stopWatch.getTime());
return res;
} catch (DataAccessException | TransactionException | PersistenceException e) {
return upsert1By1(institute, updates);
List<AccessionOpResponse> res = upsert1By1(institute, updates);
LOG.info("Processed {} accessions for {} 1by1 after {} tries in {}ms because of {}", updates.size(), instCode, tryCount + 1, stopWatch.getTime(), e.getMessage());
return res;
} catch (PleaseRetryException e) {
LOG.error("Retry {} of {} tries due to: {}", tryCount, UPLOAD_RETRIES, e.getMessage());
// Wait a bit
......
......@@ -37,6 +37,7 @@ import org.springframework.data.elasticsearch.annotations.Document;
@UniqueConstraint(name = "UQ_accession_doi", columnNames = { "doi" }) },
// Indexes
indexes = {
@Index(name = "IX_id3", columnList = "instCode,genus,acceNumb"),
@Index(name = "IX_acceNumb", columnList = "acceNumb"),
@Index(name = "IX_inst_seqNo", columnList = "instituteId,seqNo"),
@Index(name = "IX_origcty_seqNo", columnList = "orgCtyId,seqNo"),
......
......@@ -39,6 +39,7 @@ import org.genesys.blocks.model.IdUUID;
import org.genesys.blocks.model.JsonViews;
import org.genesys.blocks.model.SelfCleaning;
import org.genesys.worldclim.WorldClimUtil;
import org.genesys2.server.model.impl.AccessionIdentifier3;
import org.genesys2.server.model.impl.Country;
import org.genesys2.server.model.impl.Crop;
import org.genesys2.server.model.impl.FaoInstitute;
......@@ -56,7 +57,7 @@ import com.fasterxml.jackson.annotation.JsonUnwrapped;
import com.fasterxml.jackson.annotation.JsonView;
@MappedSuperclass
public abstract class AccessionData extends AuditedVersionedModel implements IdUUID, Serializable, SelfCleaning {
public abstract class AccessionData extends AuditedVersionedModel implements IdUUID, AccessionIdentifier3, Serializable, SelfCleaning {
private static final long serialVersionUID = -3428918862058441943L;
......@@ -222,6 +223,16 @@ public abstract class AccessionData extends AuditedVersionedModel implements IdU
public String getInstituteCode() {
return instituteCode;
}
@Override
@JsonIgnore
public String getHoldingInstitute() {
// We use this for incoming JSONs that don't have instituteCode (yet), but have institute
if (instituteCode == null && institute != null) {
return institute.getCode();
}
return instituteCode;
}
protected void setInstituteCode(final String instituteCode) {
this.instituteCode = instituteCode;
......@@ -244,6 +255,10 @@ public abstract class AccessionData extends AuditedVersionedModel implements IdU
}
public String getGenus() {
// We use this for incoming JSONs that don't have genus (yet), but have taxonomy
if (genus == null && taxonomy != null) {
return taxonomy.getGenus();
}
return genus;
}
......
......@@ -50,21 +50,13 @@ public interface AccessionRepositoryCustom {
*/
List<Accession> find(Collection<UUID> accessionUuids);
/**
* Find.
*
* @param accessions the accessions
* @return the list
*/
List<Accession> find(List<Accession> accessions);
/**
* Find by id.
*
* @param identifiers the identifiers
* @return the list
*/
List<Accession> findById(List<? extends AccessionIdentifier3> identifiers);
List<Accession> find(List<? extends AccessionIdentifier3> identifiers);
/**
* Find one.
......
......@@ -37,7 +37,6 @@ import org.genesys2.server.model.genesys.Accession;
import org.genesys2.server.model.genesys.AccessionData;
import org.genesys2.server.model.genesys.AccessionHistoric;
import org.genesys2.server.model.genesys.QAccession;
import org.genesys2.server.model.genesys.Taxonomy2;
import org.genesys2.server.model.impl.AccessionIdentifier3;
import org.genesys2.server.model.impl.FaoInstitute;
import org.genesys2.server.service.filter.AccessionFilter;
......@@ -79,7 +78,7 @@ public class AccessionRepositoryCustomImpl implements AccessionRepositoryCustom,
}
@Override
public List<Accession> find(List<Accession> forUpdate) {
public List<Accession> find(List<? extends AccessionIdentifier3> forUpdate) {
if (forUpdate == null || forUpdate.isEmpty()) {
return Collections.emptyList();
}
......@@ -89,17 +88,15 @@ public class AccessionRepositoryCustomImpl implements AccessionRepositoryCustom,
cq.distinct(true);
cq.select(root.get("id"));
Join<Accession, Taxonomy2> tax = root.join("taxonomy");
Join<Accession, FaoInstitute> inst = root.join("institute");
List<Predicate> restrictions = new ArrayList<Predicate>();
Set<String> uniqueDois = forUpdate.stream().map(aid -> aid.getDoi()).filter(doi -> doi != null).distinct().collect(Collectors.toSet());
Set<String> acceNumbs = forUpdate.stream().map(aid -> aid.getAccessionNumber()).filter(acceNumb -> acceNumb != null).distinct().collect(Collectors.toSet());
Path<Object> theDoi = root.get("doi");
Path<Object> theInstCode = inst.get("code");
Path<Object> theInstCode = root.get("instituteCode");
Path<Object> theAcceNumb = root.get("accessionNumber");
Path<Object> theGenus = tax.get("genus");
Path<Object> theGenus = root.get("genus");
if (uniqueDois.size() > 0) {
restrictions.add(theDoi.in(uniqueDois));
......@@ -107,60 +104,19 @@ public class AccessionRepositoryCustomImpl implements AccessionRepositoryCustom,
}
// A lot of .. (acceNumb=? and genus=?)
for (Accession ah : forUpdate) {
restrictions.add(criteriaBuilder.and(criteriaBuilder.equal(theInstCode, ah.getInstituteCode()), criteriaBuilder.equal(theAcceNumb, ah
.getAccessionNumber()), criteriaBuilder.equal(theGenus, ah.getTaxonomy().getGenus())));
for (AccessionIdentifier3 ah : forUpdate) {
restrictions.add(criteriaBuilder.and(criteriaBuilder.equal(theInstCode, ah.getHoldingInstitute()), criteriaBuilder.equal(theAcceNumb, ah
.getAccessionNumber()), criteriaBuilder.equal(theGenus, ah.getGenus())));
}
cq.where(criteriaBuilder.or(restrictions.toArray(EMPTY_PREDICATE_ARRAY)));
List<Long> res = em.createQuery(cq).getResultList();
if (LOG.isDebugEnabled())
LOG.trace("*** Loaded accessions {} of {}", res.size(), acceNumbs.size());
return jpaQueryFactory.selectFrom(QAccession.accession).where(QAccession.accession.id.in(res)).fetch();
}
@Override
public List<Accession> findById(List<? extends AccessionIdentifier3> identifiers) {
if (identifiers == null || identifiers.isEmpty()) {
return Collections.emptyList();
}
CriteriaQuery<Long> cq = criteriaBuilder.createQuery(Long.class);
Root<Accession> root = cq.from(Accession.class);
cq.distinct(true);
cq.select(root.get("id"));
Join<Accession, FaoInstitute> inst = root.join("institute");
Join<Accession, Taxonomy2> tax = root.join("taxonomy");
List<Predicate> restrictions = new ArrayList<Predicate>();
Set<String> uniqueDois = identifiers.stream().map(aid -> aid.getDoi()).filter(doi -> doi != null).distinct().collect(Collectors.toSet());
Set<String> acceNumbs = identifiers.stream().map(aid -> aid.getAccessionNumber()).filter(acceNumb -> acceNumb != null).distinct().collect(Collectors.toSet());
if (uniqueDois.size() > 0) {
restrictions.add(root.get("doi").in(uniqueDois));
LOG.trace("*** {} dois={}", uniqueDois);
}
Path<Object> theInstCode = inst.get("code");
Path<Object> theAcceNumb = root.get("accessionNumber");
Path<Object> theGenus = tax.get("genus");
// A lot of .. (acceNumb=? and genus=?)
for (AccessionIdentifier3 aid3 : identifiers) {
restrictions.add(criteriaBuilder.and(criteriaBuilder.equal(theInstCode, aid3.getHoldingInstitute()), criteriaBuilder.equal(theAcceNumb, aid3
.getAccessionNumber()), criteriaBuilder.equal(theGenus, aid3.getGenus())));
}
cq.where(criteriaBuilder.or(restrictions.toArray(EMPTY_PREDICATE_ARRAY)));
List<Long> res = em.createQuery(cq).getResultList();
List<Accession> res = jpaQueryFactory.selectFrom(QAccession.accession).where(QAccession.accession.id.in(em.createQuery(cq).getResultList())).fetch();
if (LOG.isDebugEnabled())
LOG.trace("*** Loaded accessions {} of {}", res.size(), acceNumbs.size());
return jpaQueryFactory.selectFrom(QAccession.accession).where(QAccession.accession.id.in(res)).fetch();
return res;
}
@Override
......
......@@ -122,7 +122,7 @@ public class AccessionServiceImpl implements AccessionService {
@Override
public Map<UUID, AccessionIdentifier3> toUUID(List<? extends AccessionIdentifier3> identifiers) {
Map<UUID, AccessionIdentifier3> res = new HashMap<>();
List<Accession> foundAccessions = accessionRepository.findById(identifiers);
List<Accession> foundAccessions = accessionRepository.find(identifiers);
for (Accession accession : foundAccessions) {
Optional<? extends AccessionIdentifier3> toPut = identifiers.stream().filter(id -> id.getAccessionNumber().equals(accession.getAccessionNumber()) && id.getGenus()
......
......@@ -542,7 +542,7 @@ public class SubsetServiceImpl implements SubsetService {
*/
private Set<AccessionRef> lookupMatchingAccessions(final Set<AccessionRef> accessionRefs) {
List<AccessionRef> list = new ArrayList<>(accessionRefs);
List<Accession> foundAccessions = accessionRepository.findById(list);
List<Accession> foundAccessions = accessionRepository.find(list);
LOG.info("Found {} matches for {} identifiers", foundAccessions.size(), accessionRefs.size());
......
......@@ -28,6 +28,7 @@ import java.util.stream.Collectors;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.time.StopWatch;
import org.apache.commons.text.WordUtils;
import org.genesys2.server.api.PleaseRetryException;
import org.genesys2.server.api.model.AccessionHeaderJson;
......@@ -66,6 +67,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.google.common.base.Stopwatch;
import com.google.common.collect.Iterators;
import javassist.util.proxy.Proxy;
......@@ -126,6 +128,7 @@ public class AccessionUploader implements InitializingBean {
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#institute, 'WRITE')")
public List<AccessionOpResponse> upsertAccessions(FaoInstitute institute, ArrayNode updates) {
assert (updates.isArray());
StopWatch stopWatch = StopWatch.createStarted();
ObjectReader reader = objectMapper.readerFor(Accession.class);
Date batchDate = new Date();
......@@ -158,8 +161,10 @@ public class AccessionUploader implements InitializingBean {
}
}
LOG.debug("Processsed incoming JSON for {} accessions in {}ms", updates.size(), stopWatch.getTime());
List<Accession> existingAccessions = accessionRepository.find(accessions);
LOG.debug("Have {} accessions for update and {} exist", accessions.size(), existingAccessions.size());
LOG.debug("Have {} accessions for update and {} exist in {}ms", accessions.size(), existingAccessions.size(), stopWatch.getTime());
for (int i = 0; i < accessions.size(); i++) {
final JsonNode update = updates.get(i);
......@@ -196,6 +201,8 @@ public class AccessionUploader implements InitializingBean {
try {
accession = applyChanges(update, updateA, accession);
LOG.trace("Applied changes to 1 accession after {}ms", stopWatch.getTime());
accession.setLastModifiedDate(batchDate);
PDCI pdci = accession.getAccessionId().getPdci();
......@@ -205,6 +212,7 @@ public class AccessionUploader implements InitializingBean {
accession.getAccessionId().setPdci(pdci);
}
PDCICalculator.updatePdci(pdci, accession);
LOG.trace("Updated PDCI to 1 accession after {}ms", stopWatch.getTime());
toSave.add(accession);
} catch (PleaseRetryException e) {
......@@ -215,9 +223,12 @@ public class AccessionUploader implements InitializingBean {
}
}
LOG.debug("Ready to update {} accessions after {}ms", toSave.size(), stopWatch.getTime());
toSave = accessionRepository.save(toSave);
LOG.trace("Saved: {}", toSave);
accessionCounter.recountInstitute(institute);
LOG.info("Upserted {} accessions in {}ms", updates.size(), stopWatch.getTime());
return responses;
}
......@@ -230,40 +241,59 @@ public class AccessionUploader implements InitializingBean {
* @return
*/
private Accession applyChanges(JsonNode update, Accession updateA, Accession accession) {
StopWatch stopWatch=StopWatch.createStarted();
update.fieldNames().forEachRemaining(fieldName -> {
LOG.trace("Updating {}", fieldName);
if ("taxonomy".equals(fieldName)) {
stopWatch.split();
updateTaxonomy(update.get("taxonomy"), updateA.getTaxonomy(), accession);
LOG.debug("Updated taxonomy for 1 accession in {}ms", stopWatch.getSplitTime());
} else if ("origCty".equals(fieldName)) {
stopWatch.split();
updateOrigCty(updateA.getOrigCty(), accession);
LOG.debug("Updated origCty for 1 accession in {}ms", stopWatch.getSplitTime());
} else if ("geo".equals(fieldName)) {
stopWatch.split();
updateGeo(update.get("geo"), updateA.getAccessionId().getGeo(), accession.getAccessionId());
LOG.debug("Updated geo for 1 accession in {}ms", stopWatch.getSplitTime());
} else if ("remarks".equals(fieldName)) {
stopWatch.split();
updateRemarks(update.get("remarks"), updateA.getAccessionId().getRemarks(), accession.getAccessionId());
LOG.debug("Updated remarks for 1 accession in {}ms", stopWatch.getSplitTime());
} else if ("coll".equals(fieldName)) {
stopWatch.split();
updateColl(update.get("coll"), updateA.getAccessionId().getColl(), accession.getAccessionId());
LOG.debug("Updated coll for 1 accession in {}ms", stopWatch.getSplitTime());
} else if ("acceName".equals(fieldName) || "otherNumb".equals(fieldName)) {
stopWatch.split();
updateAliases(fieldName, update.get(fieldName), accession.getAccessionId());
LOG.debug("Updated aliases for 1 accession in {}ms", stopWatch.getSplitTime());
} else if ("cropName".equals(fieldName)) {
stopWatch.split();
updateCrop(update.get(fieldName), accession);
LOG.debug("Updated crop for 1 accession in {}ms", stopWatch.getSplitTime());
} else if (ArrayUtils.contains(ACCESSIONID_FIELDS, fieldName)) {
try {
stopWatch.split();
copy(AccessionId.class, updateA.getAccessionId(), accession.getAccessionId(), Iterators.forArray(fieldName));
LOG.debug("Updated accessionId data for 1 accession in {}ms", stopWatch.getSplitTime());
} catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException e) {
throw new InvalidApiUsageException(e.getMessage(), e);
}
} else {
try {
stopWatch.split();
copy(Accession.class, updateA, accession, Iterators.forArray(fieldName));
LOG.debug("Updated the rest for 1 accession in {}ms", stopWatch.getSplitTime());
} catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException e) {
throw new InvalidApiUsageException(e.getMessage(), e);
}
......@@ -502,7 +532,7 @@ public class AccessionUploader implements InitializingBean {
}
final List<AccessionOpResponse> responses = new ArrayList<>(identifiers.size());
List<Accession> existingAccessions = accessionRepository.findById(identifiers);
List<Accession> existingAccessions = accessionRepository.find(identifiers);
List<Accession> toRemove = new ArrayList<>(identifiers.size());
for (int i = 0; i < identifiers.size(); i++) {
......
......@@ -4357,6 +4357,23 @@ databaseChangeLog:
CREATE INDEX UK_cus8xep8hi5fv42o3ivu0name ON accession_alias(name(250))
- changeSet:
id: 1539710994000-1
author: mobreza
comment: Index on ID3
changes:
- createIndex:
columns:
- column:
name: instCode
- column:
name: genus
- column:
name: acceNumb
indexName: IX_id3
tableName: accession
# ENABLE AFTER SOME TIME
# - changeSet:
# id: 1537463144763-folder
......
......@@ -28,6 +28,7 @@ log4j.rootLogger=error, stdout
log4j.category.org.genesys2=warn
log4j.category.org.genesys=warn
log4j.category.org.genesys2.server.api=warn
log4j.category.liquibase=debug
#log4j.category.springfox=trace
#log4j.category.org.genesys2.server.servlet.controller=debug
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment