[infinispan-dev] keySet(), values() and entrySet() not implemented

[Mircea Markus]

Manik Surtani wrote:
>
> On 4 Jun 2009, at 10:15, Galder Zamarreno wrote:
>
>>
>>
>> Mircea Markus wrote:
>>> Galder Zamarreno wrote:
>>>> Hi,
>>>>
>>>> keySet(), values(), entrySet() do not seem to be implemented at the 
>>>> moment:
>>>>
>>>>   public Set<K> keySet() {
>>>>      throw new UnsupportedOperationException("TODO implement me"); 
>>>> // TODO implement me
>>>>   }
>>>>
>>>>   public Collection<V> values() {
>>>>      throw new UnsupportedOperationException("TODO implement me"); 
>>>> // TODO implement me
>>>>   }
>>>>
>>>>   public Set<Map.Entry<K, V>> entrySet() {
>>>>      throw new UnsupportedOperationException("TODO implement me"); 
>>>> // TODO implement me
>>>>   }
>>>>
>>>> We should be implementing these methods, shouldn't we? Users might 
>>>> want to iterate the map for example to print the contents or to 
>>>> verify it contains what they put on it...etc.
>>>>
>>>> I can't see a JIRA for it, so I'll open one so that this gets 
>>>> implemented for Beta1 unless someone has any objections.
>>>>
>>>> On IRC, Mircea said it might very costly, specially for dist, but 
>>>> we still need it regardless, otherwise we're not fully implementing 
>>>> the ConcurrentMap interface and these are commonly used methods.
>>> for DIST these might be implemented as follows:
>>> -the node broadcasts a 'GetKeys' command to all others nodes in the 
>>> cluster
>>> - on remote nodes locks are acquired for all keys (not sure this 
>>> mandatory though)
>>> - each node responds with the set of keys
>>> - calling node puts them together and returns them to the user
>>> for entrySet this is even more costly, as all cluster state shall be 
>>> transfered to the calling node.
>>> Now, what worries me is the fact that users might overuse these 
>>> methods, without being aware of the performance consequences.
>>> A possible way to enforce the users to acknowledge that these 
>>> operations are costly, is to disable them by default and allow a 
>>> config element to enable them.
>>
>> I don't like the idea of having a public API method disabled by 
>> default.  Instead, I think we should warn users when calling these 
>> methods with DIST mode on.
>
> I still reckon that these methods should just query the local, 
> in-memory data container.  And be documented such that this is what is 
> expected.  Like I said the potential for OOMs is huge otherwise - 
> regardless of DIST.
that would 'lay' to the user and break the API as well. We can add Cache 
(vs Map) methods for that.
> Cheers
> -- 
> Manik Surtani
> manik at jboss.org
> Lead, Infinispan
> Lead, JBoss Cache
> http://www.infinispan.org
> http://www.jbosscache.org
>
>
>
>