Now uses a cursor in the GridFSDownload Stream #109
Conversation
Now uses a cursor rather than fetching each individual chunk JAVA-1916
| @@ -58,6 +62,16 @@ public GridFSFile getGridFSFile() { | |||
| } | |||
|
|
|||
| @Override | |||
| public GridFSDownloadStream batchSize(final int batchSize) { | |||
| if (batchSize != 0) { | |||
|
lgtm |
| @@ -33,6 +35,8 @@ | |||
| private final long length; | |||
| private final int chunkSizeInBytes; | |||
| private final int numberOfChunks; | |||
| private MongoCursor<Document> cursor; | |||
There was a problem hiding this comment.
This cursor isn't closed anywhere. It should be closed anywhere that it's set to null (if it's not already null) and in the close method of this stream (which doesn't exist yet as there wasn't a need for it previously)
| Document chunk = null; | ||
| if (cursor.hasNext()) { | ||
| chunk = cursor.next(); | ||
| if (batchSize == 1) { |
There was a problem hiding this comment.
Few questions about this bit of code:
- Why the check for batch size here? Is that because of the "special" meaning of batchSize of 1? If so, what a PITA. Note that batchSize with the new find command will mean what it should.
- Should it be responsible for validating that the "n" field equals chunkIndexToFetch? I see that's done in validateData, but that check seems to belong here instead to ensure the method contract is not violated.
There was a problem hiding this comment.
Why the check for batch size here?
Yes, it is a PITA - currently as the cursor closes, if I want to minimize memory use I have to get a new cursor each time (essentially its the same as the original way of doing find().first()).
Should it be responsible for validating that the "n" field equals chunkIndexToFetch?
I DRY'd it up but have copied it back.
|
Updated. |
|
Rebased, Squashed and merged |
The second commit adds batchSize support - so users can control memory consumption v latency.
JAVA-1916