Comments inline:
On 24 Mar 2015, at 14:22, William Burns <mudokonman(a)gmail.com>
wrote:
I am nearing completion of the new multi get command [1], allowing for
more efficient retrieval of multiple keys at the same time. Thanks
again to Radim for a lot of the initial work.
In doing so though I want to make sure I get feedback on how we want
the API to actually look, since this is much more difficult to change
down the line.
There are a few things in particular I wanted to discuss.
1. The actual name of the method. The main suggestions I have seen
are getAll or getMany. I do like the naming of the former, however it
seems a bit misleading (remember API is getAll(Set) since we are
really getting a subset. So I am thinking the possibilities at this
point are getAllOf, getAllFor or getMany. I am leaning maybe towards
the last one (getMany). I am open to any suggestions though.
JSR-107 has getAll(Set), which I think it's a bit confusing too naming wise.
getMany or getAllOf sound better to me.
2. What should the return value be for this method. Currently this
returns a Map<K, V> which makes sense when we retain these values in
the local transactional context and is pretty nice and clean for end
users.
The other idea is to use a streaming API possibly returning an
Iterable<CacheEntry<K, V>>. The streaming return value doesn't seem
as intuitive to me, but opens us up for more efficient usage
especially when there may be a lot of keys passed in (also results can
be processed concurrently while retrieval is still occurring).
I would lean towards returning the Map<K, V> value, however the next
point could change that.
3. Where this method should live. Right now this method is on the
BasicCache interface which is a parent of both Cache (embedded) and
RemoteCache (remote). Unfortunately for remote clients to be as
efficient as possible we really need a Streaming API so that we don't
have multiple copies of data in memory at multiple places at the same
time. For that reason I suggest we use the streaminig API for both or
have a different API for remote vs embedded.
Hmmm, I've been thinking about the same thing for Java 8 API (see the proposal wiki),
and I think returning a Map limits things. On Java 8, I'm considering switching
`evalAll` from returning a Map of CompletableFutures to a Stream... More to come next
week.
Now, for the current version, I think Map is limiting for both remote and embedded,
because even for embedded, retrieving an entry might be a remote operation, so that forces
you to wait for all of the entries to be available before you can return the map. Same
thing for Remote.
What about returning an Iterator? That could be lazy and more suited for the use case? If
you need to return a Map, e.g. to implement JCache.getAll(), you can wait for all and
construct a map.
Now, whether to have a separate API for this, since we have Java 8 API coming up, I'd
not worry too much at this point.
Cheers,
Let me know what you guys think.
Cheers,
- Will
[1]
https://issues.jboss.org/browse/ISPN-2183
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com