[infinispan-dev] Multi get API
Radim Vansa
rvansa at redhat.com
Thu Mar 26 10:46:24 EDT 2015
On 03/26/2015 03:00 PM, Sanne Grinovero wrote:
> On 26 March 2015 at 13:43, William Burns <mudokonman at gmail.com> wrote:
>> On Thu, Mar 26, 2015 at 8:50 AM, Sanne Grinovero <sanne at infinispan.org> wrote:
>>> In terms of naming, I'd prefer _getMulti_.
>>> My reasoning is it's nice if has a common prefix with _get_ as that
>>> helps someone exploring the API from an IDE, giving better assistance
>>> to people learning.
>>>
>>> getAll is misleading, as is getMany or get getAllOf as they imply I'm
>>> searching for something based on a pattern but you're expecting the
>>> keys and returning tuples key/value.
>> I can understand your point, however since JCache already defined this
>> as getAll I can't see but using that name.
> Ok that makes it win a bit of points, and at least it starts with _get_.
Was there any discussion regarding this during JSR-107 development? I
wonder why have they chosen such misleading name. Galder?
>
>
>>> I like the idea of returning a Map, as you can actually make the map
>>> behave like a Future: the Map instance could be returned right away,
>>> but retrieving of some entries would be blocking until they are
>>> retrieved. You wouldn't need to block on having all entries, just the
>>> ones being actively asked for. In other words, it's better than
>>> returning a Future<Map>.
>>> This should invalidate some of the previous concerns about using Map ?
>> I had thought about this briefly, but didn't give it much though since
>> it makes things quite complicated (interceptor stack and tx context I
>> am looking at you). I would say to do this later if we wanted, since
>> the API doesn't have to be different (other than the fact that people
>> should start possibly expecting CacheExceptions from calling Map.get
>> and other methods).
>>
>>> @William I didn't understand your concern on upper and lower bounds.
>>> The returned types should be coupled to the Cache types; being able to
>> The return type is coupled to the Cache types, this was about the
>> input type defined on the Set.
>>
>>> pass Object might be nice but let's make sure no casting is required
>>> on the client? Could you make some examples of your alternative
>>> proposal?
>> With Set<?> the client would never have to do any casting.
> I'm confused. Not sure which Set you're talking about. And remember
> that if you're being very liberal with the promise of returned types,
> how is the implementation going to handle the guarantee?
>
> For example, what's going to happen if the requested objects don't
> match the user expected type? The API shouldn't make promises it can't
> maintain.
The cache.get(k1) should return object stored under key k2 such that
k1.hashCode() == k2.hashCode() && k1.equals(k2)
Nothing is said about k1's type (though, Object.equals() states that
equals should always form symmetric relation).
The contract was once set for all java collections, let's stick to that.
Radim
>
> Thanks,
> Sanne
>
>>> On 26 March 2015 at 10:04, Radim Vansa <rvansa at redhat.com> wrote:
>>>> On 03/26/2015 10:28 AM, Galder Zamarreño wrote:
>>>>> Comments inline:
>>>>>
>>>>>> On 24 Mar 2015, at 14:22, William Burns <mudokonman at 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.
>>>> What's the expected usecase? I would expect that users want to just
>>>> retrieve few keys at once, without any fancy lazy
>>>> wait-for-completion-and-do-after stuff. Just imagine that you want to
>>>> pass the two values into another method, Iterator/Stream API wouldn't
>>>> help you with that much.
>>>>
>>>> It seems to me that it is very useful to have multi get with Map<K, V>
>>>> as return type in the API - we don't want to force constructs like:
>>>>
>>>> Iterator<Map.Entry<K, V>> it = cache.multiGet(key1, key2, key3);
>>>> for (; it.hasNext();) {
>>>> Map.Entry<K, V> entry = it.next();
>>>> if (entry.getKey().equals(key1)) value1 = entry.getValue();
>>>> else if (entry.getKey().equals(key2)) value2 = entry.getValue();
>>>> else if (entry.getKey().equals(key3)) value3 = entry.getValue();
>>>> }
>>>>
>>>> or let them flip it into the map manually (HashMap does not have
>>>> constructor accepting iterator)
>>>>
>>>> Iterator<Map.Entry<K, V>> it = cache.multiGet(key1, key2, key3);
>>>> Map<K, V> map = new HashMap<>();
>>>> for (; it.hasNext();) {
>>>> Map.Entry<K, V> entry = it.next();
>>>> map.put(entry.getKey(), entry.getValue());
>>>> }
>>>>
>>>>
>>>>> 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.
>>>> It seems to me that with 8, we're going to have complicated lazy
>>>> omnipotent API, and several facades simplifying that into Map, JCache or
>>>> whatever. So, let's provide the simplistic API with Map now, and the
>>>> lazy versions can get into the lazy API as soon as we'll see how the
>>>> other methods will look like.
>>>>
>>>> Radim
>>>>
>>>>> Cheers,
>>>>>
>>>>>> Let me know what you guys think.
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> - Will
>>>>>>
>>>>>> [1] https://issues.jboss.org/browse/ISPN-2183
>>>>>> _______________________________________________
>>>>>> infinispan-dev mailing list
>>>>>> infinispan-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>> --
>>>>> Galder Zamarreño
>>>>> galder at redhat.com
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> infinispan-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>
>>>> --
>>>> Radim Vansa <rvansa at redhat.com>
>>>> JBoss Performance Team
>>>>
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> infinispan-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa at redhat.com>
JBoss Performance Team
More information about the infinispan-dev
mailing list