Improve S3 Bitstream Storage to Lazy download object from S3 #9477
Conversation
… and cause - The package org.w3c.dom is accessible from more than one module: <unnamed>, java.xml
abollini
changed the title
abollini
added
the
port to dspace-7_x
| this.bufferSize = bufferSize; | ||
| } | ||
|
|
||
| public class S3LazyInputStream extends InputStream { |
There was a problem hiding this comment.
Can we add JavaDocs to describe this imbedded class?
| return byteRead; | ||
| } | ||
|
|
||
| private void downloadChunk() throws IOException, FileNotFoundException { |
There was a problem hiding this comment.
Alignment of this method is off. Can we also add a basic description to this class to describe what it's doing? This seems to be the main method.
| downloadChunk(); | ||
| } | ||
|
|
||
| int byteRead = currPos < endOfChunk ? currentChunkStream.read() : -1; |
There was a problem hiding this comment.
Code alignment it odd here. It's indented more than normal
dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java
Show resolved
Hide resolved
tdonohue
added
bug
backend: bitstore
| <groupId>xml-apis</groupId> | ||
| <artifactId>xml-apis</artifactId> | ||
| </exclusion> | ||
| </exclusions> |
There was a problem hiding this comment.
Why do we need to exclude this? How is this related to the issue this PR is solving?
There was a problem hiding this comment.
@pnbecker : This change is unrelated to the bug, but it is described above in the description of the PR:
Please note that this PR contains also an unrelated commit
2fd8680
that is needed to allow to use a recent version of Eclipse to build DSpace. Without this commit eclipse complains about The package org.w3c.dom is accessible from more than one module: , java.xml
| <groupId>xml-apis</groupId> | ||
| <artifactId>xml-apis</artifactId> | ||
| </exclusion> | ||
| </exclusions> |
There was a problem hiding this comment.
See comment at the other pom.xml
There was a problem hiding this comment.
See above comment...this change seems to be related to an issue with using Eclipse + DSpace. It is unrelated to the S3 changes but I expect @abollini made the change to allow his Eclipse IDE to work better in developing this fix.
|
I will have to look deeper into this. In a first quick test I was not able to download files anymore. I got a White label error page instead. |
Description
The
S3BitStoreService#gethas been updated to return a custom InputStream able to lazy download the underlying S3 object when used. In this way we reduce the amount of space required on the local storage during download to a configurable value (default 5Mb).Instructions for Reviewers
The
testBitstreamPutAndGetWithAlreadyPresentBucketin dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java has been updated to verify that get and put still work properly and are consistent for different content size.You should checkout the PR and try to download one or more files of different size, monitor the temp folder to verify that no files named s3-copy-... are left behind and that also downloading a large file each temp file is not larger than 5Mb.
You can eventually customize the size of the chunk in the
bitstore.xmlspring file setting the propertybufferSizein thes3Storebean definition.Checklist
pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.