Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rtirabassi uid map oom #2447

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions dcm4chee-arc-audit/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,6 @@
<version>${project.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.dcm4che.dcm4chee-arc</groupId>
<artifactId>dcm4chee-arc-keycloak</artifactId>
<version>${project.version}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.dcm4che.dcm4chee-arc</groupId>
<artifactId>dcm4chee-arc-patient</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,14 @@ private boolean hasSeriesWithOtherRejectionState(Study study, RejectionState rej
.getSingleResult() > 0;
}

public boolean hasInUseFile(String storageID, String storagePath) {
return em.createNamedQuery(Location.COUNT_IN_USE_FILES, Long.class)
.setParameter(1, storageID)
.setParameter(2, Location.Status.OK)
.setParameter(3, storagePath)
.getSingleResult() > 0;
}

private long countStudiesOfPatient(Patient patient) {
return em.createNamedQuery(Study.COUNT_STUDIES_OF_PATIENT, Long.class).setParameter(1, patient)
.getSingleResult();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,13 @@ private void deleteObjectsFromStorage(ArchiveDeviceExtension arcDev, StorageDesc
private void deleteLocation(Storage storage, Location location, AtomicInteger success, AtomicInteger skipped) {
try {
if (ejb.claimDeleteObject(location)) {
storage.deleteObject(location.getStoragePath());
if ( !ejb.hasInUseFile(location.getStorageID(), location.getStoragePath()) ) {
// File can actually be removed since not used in any other valid location
storage.deleteObject(location.getStoragePath());
}
else {
LOG.debug("Location {} deleted from {}, but file is still in use", location, storage);
}
ejb.removeLocation(location);
LOG.debug("Successfully delete {} from {}", location, storage);
success.getAndIncrement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
@Index(columnList = "multi_ref")
})
@NamedQueries({
@NamedQuery(name = Location.COUNT_IN_USE_FILES,
query = "select count(l) from Location l where l.storageID=?1 and l.status=?2 and l.storagePath=?3"),
@NamedQuery(name = Location.FIND_BY_STORAGE_ID_AND_STATUS,
query = "select l from Location l where l.storageID=?1 and l.status=?2"),
@NamedQuery(name = Location.FIND_BY_STUDY_PK,
Expand Down Expand Up @@ -125,6 +127,7 @@
})
public class Location {

public static final String COUNT_IN_USE_FILES = "Location.CountInUseFiles";
public static final String FIND_BY_STORAGE_ID_AND_STATUS = "Location.FindByStorageIDAndStatus";
public static final String FIND_BY_STUDY_PK = "Location.FindByStudyPk";
public static final String FIND_BY_SERIES_PK = "Location.FindBySeriesPk";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@
import org.dcm4chee.arc.conf.ArchiveAEExtension;
import org.dcm4chee.arc.conf.ArchiveDeviceExtension;
import org.dcm4chee.arc.entity.Series;
import org.dcm4chee.arc.metrics.MetricsService;
import org.dcm4chee.arc.entity.UIDMap;
import org.dcm4chee.arc.keycloak.HttpServletRequestInfo;
import org.dcm4chee.arc.metrics.MetricsService;
import org.dcm4chee.arc.storage.Storage;
import org.dcm4chee.arc.store.InstanceLocations;
import org.dcm4chee.arc.store.StoreService;
Expand Down Expand Up @@ -85,7 +86,7 @@ RetrieveContext newRetrieveContextGET(ArchiveAEExtension arcAE,
Association as, Attributes cmd, QueryRetrieveLevel2 qrLevel, Attributes keys);

RetrieveContext newRetrieveContextMOVE(ArchiveAEExtension arcAE,
Association as, Attributes cmd, QueryRetrieveLevel2 qrLevel, Attributes keys)
Association as, Attributes cmd, QueryRetrieveLevel2 qrLevel, Attributes keys)
throws ConfigurationException;

RetrieveContext newRetrieveContextWADO(
Expand Down Expand Up @@ -149,4 +150,6 @@ LocationInputStream openLocationInputStream(RetrieveContext ctx, InstanceLocatio
Date getLastModifiedFromMatches(RetrieveContext ctx);

Date getLastModified(RetrieveContext ctx);

UIDMap getUIDMap(Long uidMapPk, Map<Long, UIDMap> uidMapCache);
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import org.hibernate.engine.spi.SessionFactoryImplementor;

import javax.persistence.EntityManager;
import javax.persistence.NoResultException;
import javax.persistence.Tuple;
import javax.persistence.criteria.*;
import java.util.ArrayList;
Expand All @@ -82,6 +81,8 @@ class LocationQuery {
private final Path<byte[]> instanceAttrBlob;
private final List<Predicate> predicates = new ArrayList<>();
private Predicate[] iuidPredicates;
private final HashMap<Long, UIDMap> uidMapCache = new HashMap<>();


public LocationQuery(EntityManager em, CriteriaBuilder cb, RetrieveContext ctx, CodeCache codeCache) {
this.em = em;
Expand Down Expand Up @@ -138,6 +139,7 @@ public LocationQuery(EntityManager em, CriteriaBuilder cb, RetrieveContext ctx,
locationPath.get(Location_.digest),
locationPath.get(Location_.size),
locationPath.get(Location_.status),
locationPath.get(Location_.multiReference),
series.get(Series_.pk),
instance.get(Instance_.pk),
instance.get(Instance_.retrieveAETs),
Expand All @@ -146,7 +148,6 @@ public LocationQuery(EntityManager em, CriteriaBuilder cb, RetrieveContext ctx,
instance.get(Instance_.createdTime),
instance.get(Instance_.updatedTime),
uidMap.get(UIDMap_.pk),
uidMap.get(UIDMap_.encodedMap),
instanceAttrBlob
);
}
Expand All @@ -171,7 +172,7 @@ public void execute(Map<Long, StudyInfo> studyInfoMap) {
}

private void execute(Map<Long, StudyInfo> studyInfoMap, Predicate[] predicates) {
HashMap<Long,InstanceLocations> instMap = new HashMap<>();
HashMap<Long, InstanceLocations> instMap = new HashMap<>();
HashMap<Long,Attributes> seriesAttrsMap = new HashMap<>();
HashMap<Long,Map<String, CodeEntity>> rejectedInstancesOfSeriesMap = new HashMap<>();
for (Tuple tuple : em.createQuery(q.where(predicates)).getResultList()) {
Expand Down Expand Up @@ -240,9 +241,10 @@ private void addLocation(InstanceLocations match, Tuple tuple) {
.size(tuple.get(locationPath.get(Location_.size)))
.status(tuple.get(locationPath.get(Location_.status)))
.build();
location.setMultiReference(tuple.get(locationPath.get(Location_.multiReference))) ;
Long uidMapPk = tuple.get(uidMap.get(UIDMap_.pk));
if (uidMapPk != null) {
location.setUidMap(new UIDMap(uidMapPk, tuple.get(uidMap.get(UIDMap_.encodedMap))));
location.setUidMap(ctx.getRetrieveService().getUIDMap(uidMapPk, uidMapCache));
}
match.getLocations().add(location);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@
import org.dcm4chee.arc.conf.*;
import org.dcm4chee.arc.entity.Location;
import org.dcm4chee.arc.entity.Series;
import org.dcm4chee.arc.retrieve.*;
import org.dcm4chee.arc.storage.Storage;
import org.dcm4chee.arc.keycloak.HttpServletRequestInfo;
import org.dcm4chee.arc.retrieve.RetrieveContext;
import org.dcm4chee.arc.retrieve.RetrieveService;
import org.dcm4chee.arc.retrieve.SeriesInfo;
import org.dcm4chee.arc.retrieve.StudyInfo;
import org.dcm4chee.arc.storage.Storage;
import org.dcm4chee.arc.store.InstanceLocations;
import org.dcm4chee.arc.store.UpdateLocation;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@
import org.dcm4chee.arc.entity.Completeness;
import org.dcm4chee.arc.entity.Series;
import org.dcm4chee.arc.entity.Study;
import org.dcm4chee.arc.entity.UIDMap;
import org.dcm4chee.arc.retrieve.RetrieveContext;

import javax.ejb.Stateless;
import javax.persistence.EntityManager;
import javax.persistence.PersistenceContext;
import java.util.Map;

/**
* @author Gunter Zeilinger <[email protected]>
Expand Down Expand Up @@ -87,6 +89,15 @@ public void updateCompleteness(RetrieveContext ctx, Completeness completeness) {
}
}

public UIDMap getUIDMapReference(Long uidMapPk, Map<Long, UIDMap> uidMapCache) {
UIDMap uidMap = uidMapCache.get(uidMapPk) ;
if ( null == uidMap ) {
uidMap = em.find(UIDMap.class, uidMapPk) ;
uidMapCache.put(uidMapPk, uidMap) ;
}
return uidMap;
}

private void setCompletenessOfStudy(String studyInstanceUID, Completeness completeness) {
em.createNamedQuery(completeness == Completeness.PARTIAL
? Study.INCREMENT_FAILED_RETRIEVES
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@
import org.dcm4chee.arc.code.CodeCache;
import org.dcm4chee.arc.conf.*;
import org.dcm4chee.arc.entity.*;
import org.dcm4chee.arc.metrics.MetricsService;
import org.dcm4chee.arc.keycloak.HttpServletRequestInfo;
import org.dcm4chee.arc.metrics.MetricsService;
import org.dcm4chee.arc.query.scu.CFindSCU;
import org.dcm4chee.arc.query.scu.CFindSCUAttributeCoercion;
import org.dcm4chee.arc.query.util.QueryBuilder;
Expand Down Expand Up @@ -153,7 +153,7 @@ public ArchiveDeviceExtension getArchiveDeviceExtension() {

@Override
public RetrieveContext newRetrieveContextGET(ArchiveAEExtension arcAE,
Association as, Attributes rqCmd, QueryRetrieveLevel2 qrLevel, Attributes keys) {
Association as, Attributes rqCmd, QueryRetrieveLevel2 qrLevel, Attributes keys) {
RetrieveContext ctx = newRetrieveContext(arcAE, as, qrLevel, keys);
ctx.setPriority(rqCmd.getInt(Tag.Priority, 0));
ctx.setDestinationAETitle(as.getRemoteAET());
Expand All @@ -162,7 +162,7 @@ public RetrieveContext newRetrieveContextGET(ArchiveAEExtension arcAE,

@Override
public RetrieveContext newRetrieveContextMOVE(ArchiveAEExtension arcAE,
Association as, Attributes rqCmd, QueryRetrieveLevel2 qrLevel, Attributes keys)
Association as, Attributes rqCmd, QueryRetrieveLevel2 qrLevel, Attributes keys)
throws ConfigurationException {
RetrieveContext ctx = newRetrieveContext(arcAE, as, qrLevel, keys);
ctx.setPriority(rqCmd.getInt(Tag.Priority, 0));
Expand Down Expand Up @@ -321,7 +321,7 @@ public boolean calculateMatches(RetrieveContext ctx) throws DicomServiceExceptio
Collection<InstanceLocations> matches = ctx.getMatches();
matches.clear();
try {
HashMap<Long,StudyInfo> studyInfoMap = new HashMap<>();
HashMap<Long, StudyInfo> studyInfoMap = new HashMap<>();
Series.MetadataUpdate metadataUpdate = ctx.getSeriesMetadataUpdate();
if (metadataUpdate != null && metadataUpdate.instancePurgeState == Series.InstancePurgeState.PURGED) {
SeriesAttributes seriesAttributes = new SeriesAttributes(em, cb, metadataUpdate.seriesPk);
Expand Down Expand Up @@ -982,4 +982,9 @@ public BulkData createBulkData(DicomInputStream dis) throws IOException {
}
return attrs;
}

@Override
public UIDMap getUIDMap(Long uidMapPk, Map<Long, UIDMap> uidMapCache) {
return ejb.getUIDMapReference(uidMapPk, uidMapCache);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,13 @@ private Location copyLocation(StoreSession session,
Location prevLocation, Instance instance, Map<String, String> uidMap, Map<Long, UIDMap> uidMapCache) {
if (prevLocation.getMultiReference() == null) {
prevLocation = em.find(Location.class, prevLocation.getPk());
prevLocation.setMultiReference(idService.newLocationMultiReference());
// Since Location members could have been populated by a query result, not containing multiReference value,
// the location itself is now reloaded.
// This means that prevLocation.getMultiReference() can now return a non null value.
// That's why check must be done again before assigning a new value.
if (prevLocation.getMultiReference() == null) {
prevLocation.setMultiReference(idService.newLocationMultiReference());
}
}
Location newLocation = new Location(prevLocation);
newLocation.setUidMap(createUIDMap(uidMap, prevLocation.getUidMap(), uidMapCache));
Expand Down