-
Notifications
You must be signed in to change notification settings - Fork 1
Support for large downloads in Complex Portal #29
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
base: develop
Are you sure you want to change the base?
Conversation
src/main/java/uk/ac/ebi/intact/service/complex/ws/ComplexCovariationManager.java
Show resolved
Hide resolved
CompletableFuture<Void> allFutures = CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])); | ||
|
||
// The writer needs to be created before the async logic to use the right XML writer factory | ||
SerialisedComplexesWriter mainComplexWriter = ComplexWriterFactory.getSerialisedComplexesWriter(format); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the comment says, the writer needs to be created before the asynchronous call. If it isn't the XmlOutputFactory (or something like that, I don't fully remember the name) creates the wrong XmlWriter and there are issues. The classes registered are not the same at this level, then inside an asynchronous thread, which causes those issues.
List<CompletableFuture<String>> futures = new ArrayList<>(); | ||
for (List<String> complexesAcsChunk : complexesAcsChunks) { | ||
// The writer needs to be created before the async logic to use the right XML writer factory | ||
ComplexWriter complexWriter = ComplexWriterFactory.getComplexWriter(format); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
private final IntactDao intactDao; | ||
|
||
@Transactional(readOnly = true, propagation = Propagation.REQUIRED, value = "jamiTransactionManager") | ||
public String fetchAndWriteComplexes(Collection<String> complexesAcs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in a separate class to be able to use the @Transactional
annotation. With asynchronous methods and parallel threads I was getting some errors with the DB about lazy collections and no session. This method is annotated with Propagation.REQUIRED
so it ensures there's a session or creates one.
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
@Log4j | ||
@Controller | ||
@AllArgsConstructor | ||
public class SearchController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Large diff in this class, but it's mostly cleaning up and moving stuff to other classes. Now the controller is mostly jus the endpoints.
import java.io.StringWriter; | ||
import java.util.Map; | ||
|
||
public class ComplexWriterFactory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 2 types of writers.
ComplexWriter
and classes that implement it: writers that write complexes into a stringSerialisedComplexesWriter
and classes that implement it: writers that add serialised complexes strings into the right format, with the right header and footer just once.
@@ -17,16 +17,25 @@ | |||
|
|||
<Configure id="Server" class="org.eclipse.jetty.server.Server"> | |||
|
|||
<Call name="addConnector"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes related to jetty plugin update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking all good to me, really clear PR despite a complex subject. I assume you tested the performances and observed significant improvement?
It's deployed in https://wwwdev.ebi.ac.uk/intact/complex-ws/ so you can try it out if you want, or directly from the complex portal view in github pages, https://complex-portal.github.io/complex-portal-view/home, which is currently calling the dev service with no limit on the number of complexes to download. I did try a few times and if I remember correctly I managed to download all the complexes in different formats. Definitely more than the 5k limit we currently have. |
I have made changes to the Complex Portal endpoints to:
Changes are deployed in http://ves-hx-47.ebi.ac.uk:8110/intact/complex-ws/