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(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev