Reduce memory usage when returning large result sets #147
Conversation
Also includes a tiny bit of reformatting
| return results; | ||
|
|
||
| AssetCursor result = new MongoAssetCursor(cursor); | ||
| result.addOperation(CONVERT_OID_TO_HEX); |
There was a problem hiding this comment.
Both places we currently the new MongoAssetCursor, we add the convert operation. would it make sense to have a factory for this? MongoAssetCursor.newCursorWithIdConversion(cursor) or similar?
There was a problem hiding this comment.
Hmm, possibly? Or I could put this functionality into MongoAssetCursor itself on the basis that we will always need to convert the id when turning a MongoDB record into an asset.
On the other hand, it fits quite nicely into an operation.
| } | ||
|
|
||
| @Override | ||
| public void remove() { |
There was a problem hiding this comment.
Don't think there is any use-case for this, but isn't 'remove' supported by the map iterator?
There was a problem hiding this comment.
It is, but it's not supported in the MongoAssetCursor, so there doesn't seem much point implementing it in the test implementation
| * | ||
| * @return the number of assets | ||
| */ | ||
| public int getCount(); |
There was a problem hiding this comment.
any reason why you called this getCount? It seems to replace a method call to 'size' in lots of places, and wraps a call to 'size' in the case of the mongo cursor.
There was a problem hiding this comment.
Not really, getCount() made sense to me when writing it, but you're correct that other objects use size()
There was a problem hiding this comment.
For added confusion, DBCursor has length(), count() and size() which all do different things
Azquelt
force-pushed
the
stream-results
branch
from
b4dbb10
to
607b715
Compare
|
|
||
| /** {@inheritDoc} */ | ||
| @Override | ||
| public void writeTo(AssetCursor cursor, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap<String, Object> httpHeaders, |
There was a problem hiding this comment.
You're not checking the mediatype here. At this point, do we just have to assume that jax-rs has called the above method and is giving us something sensible?
There was a problem hiding this comment.
Yeah, that was what I was assuming
Newer version of Eclipse wants to reformat this class when I save, so let's separate the reformatting changes into another commit.
Azquelt
force-pushed
the
stream-results
branch
from
16f164e
to
806c167
Compare
This change allows results to be streamed from the database into a response to the client when retrieving multiple assets. It avoids needing to load the whole result set into memory before returning it. To do this we introduce the AssetCursor, which wraps a database result cursor and allows the Rest layer to iterate through the results without needing to know the details of the persistence backend. To allow changes to be made to the information from the database before it is consumed by the rest layer we introduce the AssetOperation, which encapsulates some processing which should be done to the asset before it is returned (e.g. transforming between data types or removing implementation-specific fields)
With the introduction of AssetCursor, AssetList is now only used for tests.
Azquelt
force-pushed
the
stream-results
branch
from
806c167
to
61dc2f2
Compare
Make AssetCursor implement Closable, and then read the AssetCursor within a try-with-resources so that it gets closed.
Azquelt
force-pushed
the
stream-results
branch
from
61dc2f2
to
c331e2c
Compare
|
I checked and we do have a check to ensure the search match score is not returned in the response. It's at the end of |
When returning large resultsets, stream assets one by one from the database to the response without holding the entire response in memory.