[infinispan-dev] [ISPN-78] Feedback needed

Mircea Markus mircea.markus at jboss.com
Wed Apr 13 14:08:06 EDT 2011


Really nice stuff!
On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:

> Hey Olaf,
> 
> First of all, thanks for work put so far on this! 
> 
> Pity you could not make it last week to Berlin Expert Days. I would have been great to sit down and go through this code together :| - hope you're doing better health wise :)
> 
> Here are my thoughts so far:
> 
> - First of all, since you have made several commits, it'd be nice for the rest of audience to highlight what the changes are at a high level so that people can focus on the code immediately.
+1. Also if you can issue a pull request it would be easier for to centralise or comments.
> 
> - I actually like some aspects of writeObject(key, InputStream) API cos it does the reading for the user and that can be done directly in the desired chunk size to pass it around and this is done internally, so the user has to write less code. On the other side, if we had a 'OutputStream writeToKey(K key);' the user has the freedom to decide how to read from an external stream and write to the outpustream but then we'd probably have to chunk it again to send it around the cluster cos the chunks in which the outputstream is written are not necessarily the chunks we'll write to the cluster. The only major problem I see with writeObject(key, InputStream) is if the reading the input stream blocks. So, imagine that the input stream is slow, i.e. a remote cloud provider. Then the client code is blocked and can't do anything and this is not good, so if we go with writeObject(key, InputStream), it needs to throw InterruptedException. On the other hand, the suggested code in http://community.jboss.org/wiki/LargeObjectSupport allows for the client to tweak how the input stream is read to avoid blocking, i.e. have some kind of NIO way of reading the stream.
> 
> - Rather than modifying PutKeyValueCommand, it might be better to subclass it and add the large object logic there? i.e. PutLargeKeyValueCommand - I'd also suggest adding a new build* method in the command factory...etc. This keeps the code more fine grained.
If that's the approach to be used, you can subclass directly fro AbstractDataWriteCommand which is PutKeyValueCommand's parent. That way you can have a strongly typed value as well, of type InputStream.
> 
> 
> - I don't think it's a good idea to have KeyGenerator<K> generalized. It's just complicates the code. Simply have an Object as key and it would simplify the code. Besides, you have hardcoded Chunk to a String type of chunk key and that's probably not right cos it depends on the key generator.
> 
> - Internally we don't use K of the Cache interface, we simply stick to Object to simplify the code. Externally, in the Cache interface we do keep generics but we don't use them internally. So you can remove K from LargeObjectMetadata and Chunks.
+1
> 
> Cheers,
> 
> On Apr 10, 2011, at 10:07 PM, Olaf Bergner wrote:
> 
>> I have the skeleton of an implementation of ISPN-78 - Large Object 
>> Support - at https://github.com/obergner/infinispan/tree/t_ispn78. 
>> Before going forward I could need some feedback on whether my approach 
>> makes sense at all, what alternatives there are, where things might be 
>> improved or modified to adhere to INFINISPAN's standards and so forth. 
>> Any hint is highly appreciated.
>> 
>> Keep in mind that so far I have completely ignored the issue of 
>> supporting transactions when reading and writing large objects. I would 
>> prefer to have a working core implementation before tackling the more 
>> complicated aspects.
>> 
>> Cheers,
>> Olaf
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
> 
> 
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev




More information about the infinispan-dev mailing list