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@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@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
