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

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Olaf Bergner
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
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Galder Zamarreno
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.

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

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

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
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [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
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>



_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Mircea Markus
In reply to this post by Galder Zamarreno
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
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Mircea Markus
In reply to this post by Olaf Bergner
IMO the large object methods would fit better in the Cache directly, vs AdvancedCache.
The way I see it AdvancedCache exposes to the power user internal ISPN stuff - stuff that's not generally needed by every day user. E.g. the interceptor chain, the rpc manager etc.
Everything that is plain API should be on the Cache itself, including lock() and withFlags() methods.

On 10 Apr 2011, at 21:07, 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
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani
In reply to this post by Galder Zamarreno

On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:

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

I don't understand why you need a specialised PutKVCommand?  How is behaviour different?

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani
In reply to this post by Mircea Markus

On 13 Apr 2011, at 19:26, Mircea Markus wrote:

> IMO the large object methods would fit better in the Cache directly, vs AdvancedCache.

Perhaps a new interface?  StreamingCache?

> The way I see it AdvancedCache exposes to the power user internal ISPN stuff - stuff that's not generally needed by every day user. E.g. the interceptor chain, the rpc manager etc.
> Everything that is plain API should be on the Cache itself, including lock() and withFlags() methods.

Not really - I'd rather leave these as they are since they are considered advanced usage methods which require understanding of how Infinispan works internally.

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani
In reply to this post by Olaf Bergner

On 10 Apr 2011, at 21:07, Olaf Bergner wrote:

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


How do you maintain consistency without transactions?  E.g., Concurrent readers and a writer?

Concurrent writers isn't a problem since we don't support this, however we may be able to add some kind of concurrent write support if we consider the streams as append-only.

Cheers
Manik

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Olaf Bergner
Am 15.04.11 17:01, schrieb Manik Surtani:

> On 10 Apr 2011, at 21:07, Olaf Bergner wrote:
>
>> 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.
>
> How do you maintain consistency without transactions?  E.g., Concurrent readers and a writer?
>
> Concurrent writers isn't a problem since we don't support this, however we may be able to add some kind of concurrent write support if we consider the streams as append-only.
I never meant to actually *publish* large object support without
mechanisms ensuring consistency in place. Yet I would prefer to have
most of the other issues - what should the official API's first
iteration look like?, is the approach I've taken so far basically sane?
and so forth - sorted before moving on to the more complicated aspects.
So far, it took some time to get used to the code base and acquire
*some* knowledge about INFINISPAN's inner workings, but it hasn't been
exactly rocket science. Just a lot of work. Ensuring consistency,
however, won't probably be that easy. Especially since I only have a
very shallow understanding of INFINISPAN's transaction support's inner
workings.

> Cheers
> Manik
>
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Olaf Bergner
In reply to this post by Manik Surtani
Am 15.04.11 16:59, schrieb Manik Surtani:
> On 13 Apr 2011, at 19:26, Mircea Markus wrote:
>
>> IMO the large object methods would fit better in the Cache directly, vs AdvancedCache.
> Perhaps a new interface?  StreamingCache?
I do like this approach. It keeps closely related methods in one place
and might help to mitigate uncontrolled growth of the Cache interface.

Cheers,
Olaf

>> The way I see it AdvancedCache exposes to the power user internal ISPN stuff - stuff that's not generally needed by every day user. E.g. the interceptor chain, the rpc manager etc.
>> Everything that is plain API should be on the Cache itself, including lock() and withFlags() methods.
> Not really - I'd rather leave these as they are since they are considered advanced usage methods which require understanding of how Infinispan works internally.
>
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Olaf Bergner
In reply to this post by Manik Surtani
Am 15.04.11 16:56, schrieb Manik Surtani:
> On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:
>
>> - 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.
> I don't understand why you need a specialised PutKVCommand?  How is behaviour different?
In its present shape PutKVCommand will *always* attempt to read the
given key's value before proceeding to store the new value. I doubt that
we need this for large objects. To determine if we are to perform a
create or an update it suffices to consult the metadata cache. We won't
be returning the former value anyway.

Cheers,
Olaf

> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani
In reply to this post by Olaf Bergner

On 15 Apr 2011, at 17:53, Olaf Bergner wrote:

> Am 15.04.11 17:01, schrieb Manik Surtani:
>> On 10 Apr 2011, at 21:07, Olaf Bergner wrote:
>>
>>> 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.
>>
>> How do you maintain consistency without transactions?  E.g., Concurrent readers and a writer?
>>
>> Concurrent writers isn't a problem since we don't support this, however we may be able to add some kind of concurrent write support if we consider the streams as append-only.
> I never meant to actually *publish* large object support without
> mechanisms ensuring consistency in place.

Of course - I wasn't suggesting that stuff was fully baked, I was just curious as to your plans on next steps, etc.  :-)

> Yet I would prefer to have
> most of the other issues - what should the official API's first
> iteration look like?, is the approach I've taken so far basically sane?
> and so forth - sorted before moving on to the more complicated aspects.
> So far, it took some time to get used to the code base and acquire
> *some* knowledge about INFINISPAN's inner workings, but it hasn't been
> exactly rocket science. Just a lot of work. Ensuring consistency,
> however, won't probably be that easy. Especially since I only have a
> very shallow understanding of INFINISPAN's transaction support's inner
> workings.

Mircea will be able to help you on this.  I think he has some slides on the subject that he's preparing to present at JUDCon - I'm sure he'll be happy to share them with you.  :-)

Cheers
Manik
--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani
In reply to this post by Olaf Bergner

On 15 Apr 2011, at 17:56, Olaf Bergner wrote:

> Am 15.04.11 16:59, schrieb Manik Surtani:
>> On 13 Apr 2011, at 19:26, Mircea Markus wrote:
>>
>>> IMO the large object methods would fit better in the Cache directly, vs AdvancedCache.
>> Perhaps a new interface?  StreamingCache?
> I do like this approach. It keeps closely related methods in one place
> and might help to mitigate uncontrolled growth of the Cache interface.

Lets stick with this then.  :-)

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani
In reply to this post by Olaf Bergner

On 15 Apr 2011, at 18:00, Olaf Bergner wrote:

Am 15.04.11 16:56, schrieb Manik Surtani:
On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:

- 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.
I don't understand why you need a specialised PutKVCommand?  How is behaviour different?
In its present shape PutKVCommand will *always* attempt to read the
given key's value before proceeding to store the new value. I doubt that
we need this for large objects. To determine if we are to perform a
create or an update it suffices to consult the metadata cache. We won't
be returning the former value anyway.

Understood.  Even locking semantics would be different since you only acquire the lock on the metadata entry and not on each and every chunk.  Correct?

So I presume you are building this as a separate Maven module, and making use of the module-specific commands as described here [1] and [2] ?

Cheers
Manik

--


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Galder Zamarreno

On Apr 18, 2011, at 12:35 PM, Manik Surtani wrote:

>
> On 15 Apr 2011, at 18:00, Olaf Bergner wrote:
>
>> Am 15.04.11 16:56, schrieb Manik Surtani:
>>> On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:
>>>
>>>> - 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.
>>> I don't understand why you need a specialised PutKVCommand?  How is behaviour different?
>> In its present shape PutKVCommand will *always* attempt to read the
>> given key's value before proceeding to store the new value. I doubt that
>> we need this for large objects. To determine if we are to perform a
>> create or an update it suffices to consult the metadata cache. We won't
>> be returning the former value anyway.
>
> Understood.  Even locking semantics would be different since you only acquire the lock on the metadata entry and not on each and every chunk.  Correct?
>
> So I presume you are building this as a separate Maven module, and making use of the module-specific commands as described here [1] and [2] ?

Why would he need a separate module? He's not bringing any new dependencies AFAIK

>
> Cheers
> Manik
>
> [1] https://issues.jboss.org/browse/ISPN-256?focusedCommentId=12586154#comment-12586154
> [2] https://github.com/infinispan/infinispan-sample-module 
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Olaf Bergner

-------- Original-Nachricht --------
> Datum: Tue, 19 Apr 2011 11:06:50 +0200
> Von: "Galder Zamarreño" <[hidden email]>
> An: infinispan -Dev List <[hidden email]>
> Betreff: Re: [infinispan-dev] [ISPN-78] Feedback needed

>
> On Apr 18, 2011, at 12:35 PM, Manik Surtani wrote:
>
> >
> > On 15 Apr 2011, at 18:00, Olaf Bergner wrote:
> >
> >> Am 15.04.11 16:56, schrieb Manik Surtani:
> >>> On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:
> >>>
> >>>> - 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.
> >>> I don't understand why you need a specialised PutKVCommand?  How is
> behaviour different?
> >> In its present shape PutKVCommand will *always* attempt to read the
> >> given key's value before proceeding to store the new value. I doubt
> that
> >> we need this for large objects. To determine if we are to perform a
> >> create or an update it suffices to consult the metadata cache. We won't
> >> be returning the former value anyway.
> >
> > Understood.  Even locking semantics would be different since you only
> acquire the lock on the metadata entry and not on each and every chunk.
> Correct?
> >

As to locking I currently think that locking metadata will probably suffice. Right now I started thinking about how to eventually implement transactions. Currently a transactions accumulated state is retained until it is finally prepared/committed, correct? When involving large objects this may well blow up the originator node's cache. I thought about preparing transaction branches storing chunks immediately (without waiting for the transaction to complete but haven't yet thought about the implications. This might require some eager locking, though. Any thoughts on this.

As to reusing existing commands instead of creating new ones: I originally intended to do so yet the deeper into implementing I got the more I came to the conclusion that a custom command would be more appropriate. The alternative would be to parameterize an existing command, effectively telling it that it is dealing with a large object. This might get messy if more and more requirements need to be implemented.


> > So I presume you are building this as a separate Maven module, and
> making use of the module-specific commands as described here [1] and [2] ?
>
> Why would he need a separate module? He's not bringing any new
> dependencies AFAIK
>

I wasn't aware of this possibility. It should be relatively easy to port my current solution to that extension mechanism if the need arises.

Cheers,
Olaf

> >
> > Cheers
> > Manik
> >
> > [1]
> https://issues.jboss.org/browse/ISPN-256?focusedCommentId=12586154#comment-12586154
> > [2] https://github.com/infinispan/infinispan-sample-module 
> > --
> > Manik Surtani
> > [hidden email]
> > twitter.com/maniksurtani
> >
> > Lead, Infinispan
> > http://www.infinispan.org
> >
> >
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > [hidden email]
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
NEU: FreePhone - kostenlos mobil telefonieren und surfen!
Jetzt informieren: http://www.gmx.net/de/go/freephone
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Manik Surtani

On 19 Apr 2011, at 12:05, Olaf Bergner wrote:

> As to locking I currently think that locking metadata will probably suffice. Right now I started thinking about how to eventually implement transactions. Currently a transactions accumulated state is retained until it is finally prepared/committed, correct? When involving large objects this may well blow up the originator node's cache. I thought about preparing transaction branches storing chunks immediately (without waiting for the transaction to complete but haven't yet thought about the implications. This might require some eager locking, though. Any thoughts on this.

Well, if you store a chunk before the tx completes, handling a rollback could be tough.  Eager locking too would need to change to also push the modification eagerly.  Vladimir should have some thoughts around this.

> As to reusing existing commands instead of creating new ones: I originally intended to do so yet the deeper into implementing I got the more I came to the conclusion that a custom command would be more appropriate. The alternative would be to parameterize an existing command, effectively telling it that it is dealing with a large object. This might get messy if more and more requirements need to be implemented.

Pros and cons.  Much cleaner to have your own command (or a subclass), I agree.  But it means much more visitor methods which can get messy.  Hence my suggestion to use a separate module.  But this isn't that important for now, as you point out.

Cheers
Manik
--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Mircea Markus
In reply to this post by Galder Zamarreno

On 19 Apr 2011, at 10:06, Galder Zamarreño wrote:

>
> On Apr 18, 2011, at 12:35 PM, Manik Surtani wrote:
>
>>
>> On 15 Apr 2011, at 18:00, Olaf Bergner wrote:
>>
>>> Am 15.04.11 16:56, schrieb Manik Surtani:
>>>> On 11 Apr 2011, at 13:13, Galder Zamarreño wrote:
>>>>
>>>>> - 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.
>>>> I don't understand why you need a specialised PutKVCommand?  How is behaviour different?
>>> In its present shape PutKVCommand will *always* attempt to read the
>>> given key's value before proceeding to store the new value. I doubt that
>>> we need this for large objects. To determine if we are to perform a
>>> create or an update it suffices to consult the metadata cache. We won't
>>> be returning the former value anyway.
>>
>> Understood.  Even locking semantics would be different since you only acquire the lock on the metadata entry and not on each and every chunk.  Correct?
>>
>> So I presume you are building this as a separate Maven module, and making use of the module-specific commands as described here [1] and [2] ?
>
> Why would he need a separate module? He's not bringing any new dependencies AFAIK
Also large objects don't look orthogonal to the existing functionality ( e.g. like the server are) but more like a core thing, like e.g. explicit locking.
OTOH I think that these (large objects, eager locking) are features to be used by "power users" and not necessarily to be exposed on the o.i.Cache interface. So an interface within the same module, perhaps even extending o.i.Cache would make more sense to me.




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Olaf Bergner
Am 19.04.11 16:59, schrieb Mircea Markus:
> On 19 Apr 2011, at 10:06, Galder Zamarreño wrote:
> So I presume you are building this as a separate Maven module, and making use of the module-specific commands as described here [1] and [2] ?
>> Why would he need a separate module? He's not bringing any new dependencies AFAIK
> Also large objects don't look orthogonal to the existing functionality ( e.g. like the server are) but more like a core thing, like e.g. explicit locking.
> OTOH I think that these (large objects, eager locking) are features to be used by "power users" and not necessarily to be exposed on the o.i.Cache interface. So an interface within the same module, perhaps even extending o.i.Cache would make more sense to me.
>
I personally favor a separate dedicated interface, e.g. StreamingCache
as has been suggested, and maybe a method getStreamingCache()  on Cache.
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

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

Mircea Markus

On 19 Apr 2011, at 18:17, Olaf Bergner wrote:

> Am 19.04.11 16:59, schrieb Mircea Markus:
>> On 19 Apr 2011, at 10:06, Galder Zamarreño wrote:
>> So I presume you are building this as a separate Maven module, and making use of the module-specific commands as described here [1] and [2] ?
>>> Why would he need a separate module? He's not bringing any new dependencies AFAIK
>> Also large objects don't look orthogonal to the existing functionality ( e.g. like the server are) but more like a core thing, like e.g. explicit locking.
>> OTOH I think that these (large objects, eager locking) are features to be used by "power users" and not necessarily to be exposed on the o.i.Cache interface. So an interface within the same module, perhaps even extending o.i.Cache would make more sense to me.
>>
> I personally favor a separate dedicated interface, e.g. StreamingCache
> as has been suggested, and maybe a method getStreamingCache()  on Cache.
yes, that would do it indeed. Just not sure about the name: as long as you have Cache in it users might expect to extend the Cache interface. StreamingHandler perhaps?

>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
12