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

[Manik Surtani]

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.

Cheers
--
Manik Surtani
manik at jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org