Avoid a deadlock uploading terminology in postgres (#2529)

* Fix #2493

* Try to fix intermittent test failure
This commit is contained in:
James Agnew 2021-04-08 12:28:42 -04:00 committed by GitHub
parent 462d9bc6c4
commit f70648484a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 15 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 2493
title: "A database deadlock in Postgresql was observed when uploading large terminology CodeSystems using
deferred uploading. Thanks to Tyge Folke Nielsen for reporting and suggesting a fix!"

View File

@ -60,7 +60,9 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Queue;
import java.util.UUID;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
@ -68,7 +70,7 @@ public class TermDeferredStorageSvcImpl implements ITermDeferredStorageSvc {
private static final Logger ourLog = LoggerFactory.getLogger(TermDeferredStorageSvcImpl.class);
final private List<TermCodeSystem> myDeferredCodeSystemsDeletions = Collections.synchronizedList(new ArrayList<>());
final private List<TermCodeSystemVersion> myDeferredCodeSystemVersionsDeletions = Collections.synchronizedList(new ArrayList<>());
final private Queue<TermCodeSystemVersion> myDeferredCodeSystemVersionsDeletions = new ConcurrentLinkedQueue<>();
final private List<TermConcept> myDeferredConcepts = Collections.synchronizedList(new ArrayList<>());
final private List<ValueSet> myDeferredValueSets = Collections.synchronizedList(new ArrayList<>());
final private List<ConceptMap> myDeferredConceptMaps = Collections.synchronizedList(new ArrayList<>());
@ -498,7 +500,7 @@ public class TermDeferredStorageSvcImpl implements ITermDeferredStorageSvc {
}
@Override
public synchronized void deleteCodeSystemVersion(TermCodeSystemVersion theCodeSystemVersion) {
public void deleteCodeSystemVersion(TermCodeSystemVersion theCodeSystemVersion) {
myDeferredCodeSystemVersionsDeletions.add(theCodeSystemVersion);
}

View File

@ -1,35 +1,50 @@
package ca.uhn.fhir.rest.server.method;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.FhirVersionEnum;
import ca.uhn.fhir.rest.annotation.Metadata;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.server.IRestfulServer;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import com.google.common.collect.Lists;
import org.hl7.fhir.instance.model.api.IBaseConformance;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import javax.servlet.http.HttpServletRequest;
import java.lang.reflect.Method;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
public class ConformanceMethodBindingTest {
private FhirContext fhirContext;
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
protected ServletRequestDetails mySrd;
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private FhirContext myFhirContext;
private ConformanceMethodBinding conformanceMethodBinding;
@BeforeEach
public void setUp() {
fhirContext = mock(FhirContext.class);
}
private <T> T init(T theCapabilityStatementProvider) throws NoSuchMethodException {
T provider = spy(theCapabilityStatementProvider);
Method method = provider.getClass().getDeclaredMethod("getServerConformance", HttpServletRequest.class, RequestDetails.class);
conformanceMethodBinding = new ConformanceMethodBinding(method, fhirContext, provider);
conformanceMethodBinding = new ConformanceMethodBinding(method, myFhirContext, provider);
return provider;
}
@ -37,9 +52,9 @@ public class ConformanceMethodBindingTest {
public void invokeServerCached() throws NoSuchMethodException {
TestResourceProvider provider = init(new TestResourceProvider());
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());
}
@ -47,12 +62,12 @@ public class ConformanceMethodBindingTest {
public void invokeServerCacheExpires() throws NoSuchMethodException {
TestResourceProviderSmallCache provider = init(new TestResourceProviderSmallCache());
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());
sleepAtLeast(20);
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, timeout(10000).times(2)).getServerConformance(any(), any());
}
@ -61,10 +76,10 @@ public class ConformanceMethodBindingTest {
public void invokeServerCacheDisabled() throws NoSuchMethodException {
TestResourceProviderNoCache provider = init(new TestResourceProviderNoCache());
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(2)).getServerConformance(any(), any());
}
@ -72,11 +87,11 @@ public class ConformanceMethodBindingTest {
public void invokeServerCacheDisabledInSuperclass() throws NoSuchMethodException {
TestResourceProviderNoCache2 provider = init(new TestResourceProviderNoCache2());
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());
// We currently don't scan the annotation on the superclass...Perhaps we should
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mock(RequestDetails.class, RETURNS_DEEP_STUBS), new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), mySrd, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());
}
@ -84,7 +99,7 @@ public class ConformanceMethodBindingTest {
public void invokeServerNotCached_ClientControlled() throws NoSuchMethodException {
TestResourceProvider provider = init(new TestResourceProvider());
RequestDetails requestDetails = mock(RequestDetails.class, RETURNS_DEEP_STUBS);
RequestDetails requestDetails = mySrd;
when(requestDetails.getHeaders(Constants.HEADER_CACHE_CONTROL)).thenReturn(Lists.newArrayList(Constants.CACHE_CONTROL_NO_CACHE));
conformanceMethodBinding.invokeServer(mock(IRestfulServer.class, RETURNS_DEEP_STUBS), requestDetails, new Object[]{mock(HttpServletRequest.class), mock(RequestDetails.class)});
verify(provider, times(1)).getServerConformance(any(), any());