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

[Olaf Bergner]

Hi Galder,

Am 11.04.11 14:13, schrieb Galder Zamarreño:
> 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 :)
>
Yes, it would have been nice to sit down together and go through the 
code together. It wasn't meant to be. Meanwhile, I'm getting better 
slowly, but I do.
> 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.
Makes sense. So without further ado:

(A) writeToKey(K key, InputStream largeObject)

     1. Added writeToKey(K key, InputStream largeObject) to AdvancedCache.
     2. Added property isPutLargeObject to PutKeyValueCommand so that 
interceptors down the call stack know what they are dealing with.
     3. Implemented writeToKey(K, InputStream) in CacheDelegate where it 
creates a PutKeyValueCommand and sets its isPutLargeObject property to true.
     4. Introduced LargeObjectSupportInterceptor responsible for (a) 
partitioning large objects into chunks, (b) storing each chunk and (c) 
storing a large object's metadata once it has been written to the cache.
     5. For partitioning a large object, LargeObjectSupportInterceptor 
delegates to org.infinispan.largeobjectsupport.Chunks.
     6. A large object's metadata are represented by 
org.infinispan.largeobjectsupport.LargeObjectMetadata. Holds the large 
object's key, its total size, the maximum chunk size in bytes and an 
array of chunk keys.
     7. Introduced 
org.infinispan.largeobjectsupport.LargeObjectMetadataManager(Impl) and a 
corresponding factory. LargeObjectMetadataManager acts as a facade for a 
dedicated cache that stores LargeObjectMetadata keyed on large object keys.

(B) InputStream readFromKey(K key)

     1. Added InputStream readFromKey(K key) to AdvancedCache.
     2. Introduced LargeObjectInputStream. Takes a LargeObjectMetadata 
instance and a cache reference at construction time and knows how to 
retrieve the requested large object chunk-wise from the given cache.
     3. Implemented InputStream readFromKey(K) in CacheDelegate where it 
uses the LargeObjectMetadataManager to look up the corresponding 
LargeObjectMetadata and returns a LargeObjectInputStream.
> - 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.
I plan on eventually supporting both but wanted to start with the easier 
one of the two.
> - 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.
Actually, I started out having my own WriteLargeObjectCommand (though it 
wasn't a subclass of PutKeyValueCommand) but someone on this list 
suggested to reuse PutKeyValueCommand. I personally prefer to have a 
dedicated command, and so I will follow your advice.
> - 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.
I'll remove the generics.

Thanks for the feedback,

Olaf
> 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
>