[infinispan-dev] Multi get API
Galder Zamarreño
galder at redhat.com
Tue Mar 31 11:25:41 EDT 2015
> On 26 Mar 2015, at 15:46, Radim Vansa <rvansa at redhat.com> wrote:
>
> 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?
Dunno, never saw it discussed, but I supposed they chose that name cos of Map.putAll
I think it's the wrong name, getMany, getMulti are better IMO, regardless of what JCache/java.util.Map do.
Also, see what they did with CacheWriter and CacheLoader, they're no symmetrical... and we followed the same pattern in our persistence :|
>
>>
>>
>>>> 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
>
> _______________________________________________
> 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
More information about the infinispan-dev
mailing list