Quantcast

[infinispan-dev] Cache Store Marshalling

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[infinispan-dev] Cache Store Marshalling

Ryan Emerson-2
Hi All,

Currently the CacheWriterInterceptor utilises the internal marshaller for marshalling entries before they are sent to the configured cache stores. This causes several problems [1], most notably that changes to the internal marshaller make stored data incompatible across Infinispan versions.

I propose that we decouple the internal and store marshaller. To allow 9.x versions to remain compatible, we should default to the internal marshaller (until 10.x at least), but optionally allow users to specify a custom StreamingMarshaller implementation as part of their PersistenceConfiguration. As we already have the protostuff and kryo bridges, users would specify an enum for the marshaller they want to use as well as an optional class string if a custom implementation is required. So for example:

enum StoreMarshaller {
   CUSTOM,
   INTERNAL,
   KRYO
   PROTOSTUFF;
}

new ConfigurationBuilder()
    .persistence()
        .marshaller(StoreMarshaller.CUSTOM)
        .marshallerClass("org.example.Marshaller")
         ...

Finally, Gustavo brought flatbuffers[2] to my attention which could be a good option to provide for users as one of the default StoreMarshaller implementations.

WDYT?

Cheers
Ryan

[1] https://docs.google.com/document/d/1PR0eYgjhqXUR5w03npS7TdOs2KDZjfdMh0au8XAsyJY
[2] https://google.github.io/flatbuffers/
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Cache Store Marshalling

Dan Berindei
+100 to use a separate marshaller for user types, but I think the
persistence configuration is the wrong place for it. Users shouldn't
have to implement an AdvancedExternalizer for efficient replication
between nodes if they already have a marshaller that's efficient
enough for use in a store.

I would have liked the default user type marshaller to use
jboss-marshalling with the all the 8.2.x internal externalizers, in
order to have backwards compatibility for the stores without needing
an upgrade tool. But it seems like the stores change their layout all
the time anyway, so I would settle for some version metadata in the
store and better error messages instead.

I'm not sure about the enum, don't like the feeling that marshallers
not bundled with the core are somehow second-class. I'd rather have a
single marshaller name string attribute, have a NAME constant in each
marshaller class, and register the names on startup, either via
ServiceLoader or via ModuleLifecycle.

There's also the question of whether we should support passing in a
marshaller instance instead of a class name (I think we should, at
least for testing) and whether we should allow custom configuration
attributes/elements for the marshaller in the XML configuration (no
idea).

Cheers
Dan



On Tue, Mar 14, 2017 at 1:26 PM, Ryan Emerson <[hidden email]> wrote:

> Hi All,
>
> Currently the CacheWriterInterceptor utilises the internal marshaller for marshalling entries before they are sent to the configured cache stores. This causes several problems [1], most notably that changes to the internal marshaller make stored data incompatible across Infinispan versions.
>
> I propose that we decouple the internal and store marshaller. To allow 9.x versions to remain compatible, we should default to the internal marshaller (until 10.x at least), but optionally allow users to specify a custom StreamingMarshaller implementation as part of their PersistenceConfiguration. As we already have the protostuff and kryo bridges, users would specify an enum for the marshaller they want to use as well as an optional class string if a custom implementation is required. So for example:
>
> enum StoreMarshaller {
>    CUSTOM,
>    INTERNAL,
>    KRYO
>    PROTOSTUFF;
> }
>
> new ConfigurationBuilder()
>     .persistence()
>         .marshaller(StoreMarshaller.CUSTOM)
>         .marshallerClass("org.example.Marshaller")
>          ...
>
> Finally, Gustavo brought flatbuffers[2] to my attention which could be a good option to provide for users as one of the default StoreMarshaller implementations.
>
> WDYT?
>
> Cheers
> Ryan
>
> [1] https://docs.google.com/document/d/1PR0eYgjhqXUR5w03npS7TdOs2KDZjfdMh0au8XAsyJY
> [2] https://google.github.io/flatbuffers/
> _______________________________________________
> 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
|  
Report Content as Inappropriate

Re: [infinispan-dev] Cache Store Marshalling

Ryan Emerson-2
Comments inline.

----- Original Message -----
> +100 to use a separate marshaller for user types, but I think the
> persistence configuration is the wrong place for it. Users shouldn't
> have to implement an AdvancedExternalizer for efficient replication
> between nodes if they already have a marshaller that's efficient
> enough for use in a store.

When I sent the original email, I was thinking that we would allow users to specify a marshaller that was specific to persistence, in the same way that we allow users to configure a compatibility marshaller. This way users could utilise a serialisation library better suited to their performance needs, i.e. faster deserializaton vs disk space etc. But I don't have a specific use case in mind, so this might be overkill.

Regarding a generic user marshaller, Galder has already created ISPN-7409 [1]. Galder, do you intend to work on this/have a specific release in mind? I noticed in the planning doc that it hasn't been marked for any release.

[1] https://issues.jboss.org/browse/ISPN-7409

> I would have liked the default user type marshaller to use
> jboss-marshalling with the all the 8.2.x internal externalizers, in
> order to have backwards compatibility for the stores without needing
> an upgrade tool. But it seems like the stores change their layout all
> the time anyway, so I would settle for some version metadata in the
> store and better error messages instead.

+1 to adding some version metadata.

> I'm not sure about the enum, don't like the feeling that marshallers
> not bundled with the core are somehow second-class. I'd rather have a
> single marshaller name string attribute, have a NAME constant in each
> marshaller class, and register the names on startup, either via
> ServiceLoader or via ModuleLifecycle.
>
> There's also the question of whether we should support passing in a
> marshaller instance instead of a class name (I think we should, at
> least for testing) and whether we should allow custom configuration
> attributes/elements for the marshaller in the XML configuration (no
> idea).

All sounds good.

> Cheers
> Dan
>
>
>
> On Tue, Mar 14, 2017 at 1:26 PM, Ryan Emerson <[hidden email]> wrote:
> > Hi All,
> >
> > Currently the CacheWriterInterceptor utilises the internal marshaller for
> > marshalling entries before they are sent to the configured cache stores.
> > This causes several problems [1], most notably that changes to the
> > internal marshaller make stored data incompatible across Infinispan
> > versions.
> >
> > I propose that we decouple the internal and store marshaller. To allow 9.x
> > versions to remain compatible, we should default to the internal
> > marshaller (until 10.x at least), but optionally allow users to specify a
> > custom StreamingMarshaller implementation as part of their
> > PersistenceConfiguration. As we already have the protostuff and kryo
> > bridges, users would specify an enum for the marshaller they want to use
> > as well as an optional class string if a custom implementation is
> > required. So for example:
> >
> > enum StoreMarshaller {
> >    CUSTOM,
> >    INTERNAL,
> >    KRYO
> >    PROTOSTUFF;
> > }
> >
> > new ConfigurationBuilder()
> >     .persistence()
> >         .marshaller(StoreMarshaller.CUSTOM)
> >         .marshallerClass("org.example.Marshaller")
> >          ...
> >
> > Finally, Gustavo brought flatbuffers[2] to my attention which could be a
> > good option to provide for users as one of the default StoreMarshaller
> > implementations.
> >
> > WDYT?
> >
> > Cheers
> > Ryan
> >
> > [1]
> > https://docs.google.com/document/d/1PR0eYgjhqXUR5w03npS7TdOs2KDZjfdMh0au8XAsyJY
> > [2] https://google.github.io/flatbuffers/
> > _______________________________________________
> > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Cache Store Marshalling

Galder Zamarreño
In reply to this post by Ryan Emerson-2
Hmmm, we've already discussed moving from the current internal/external marshaller architecture, where even user types are marshalled with internal marshaller, to an internal/user marshaller architecture where user types are only marshalled by the user marshaller. This is mentioned in [1] and the switch to internal/user marshaller is already being tracked in [3].

Your suggestion feels like a subset of what [3] is trying to achieve. In [3], the idea is not only to use the user marshaller for clustering but of course, for persistence.

Cheers,

[3] https://issues.jboss.org/browse/ISPN-7409
--
Galder Zamarreño
Infinispan, Red Hat

> On 14 Mar 2017, at 12:26, Ryan Emerson <[hidden email]> wrote:
>
> Hi All,
>
> Currently the CacheWriterInterceptor utilises the internal marshaller for marshalling entries before they are sent to the configured cache stores. This causes several problems [1], most notably that changes to the internal marshaller make stored data incompatible across Infinispan versions.
>
> I propose that we decouple the internal and store marshaller. To allow 9.x versions to remain compatible, we should default to the internal marshaller (until 10.x at least), but optionally allow users to specify a custom StreamingMarshaller implementation as part of their PersistenceConfiguration. As we already have the protostuff and kryo bridges, users would specify an enum for the marshaller they want to use as well as an optional class string if a custom implementation is required. So for example:
>
> enum StoreMarshaller {
>   CUSTOM,
>   INTERNAL,
>   KRYO
>   PROTOSTUFF;
> }
>
> new ConfigurationBuilder()
>    .persistence()
>        .marshaller(StoreMarshaller.CUSTOM)
>        .marshallerClass("org.example.Marshaller")
>         ...
>
> Finally, Gustavo brought flatbuffers[2] to my attention which could be a good option to provide for users as one of the default StoreMarshaller implementations.
>
> WDYT?
>
> Cheers
> Ryan
>
> [1] https://docs.google.com/document/d/1PR0eYgjhqXUR5w03npS7TdOs2KDZjfdMh0au8XAsyJY
> [2] https://google.github.io/flatbuffers/
> _______________________________________________
> 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
|  
Report Content as Inappropriate

Re: [infinispan-dev] Cache Store Marshalling

Galder Zamarreño
In reply to this post by Ryan Emerson-2
Inline:

> On 15 Mar 2017, at 10:59, Ryan Emerson <[hidden email]> wrote:
>
> Comments inline.
>
> ----- Original Message -----
>> +100 to use a separate marshaller for user types, but I think the
>> persistence configuration is the wrong place for it. Users shouldn't
>> have to implement an AdvancedExternalizer for efficient replication
>> between nodes if they already have a marshaller that's efficient
>> enough for use in a store.
>
> When I sent the original email, I was thinking that we would allow users to specify a marshaller that was specific to persistence, in the same way that we allow users to configure a compatibility marshaller. This way users could utilise a serialisation library better suited to their performance needs, i.e. faster deserializaton vs disk space etc. But I don't have a specific use case in mind, so this might be overkill.

Compatibility mode (and compatibility marshaller stuff) will eventually go in favour of transcoding, so I would not consider it a good way of doing things ;)

>
> Regarding a generic user marshaller, Galder has already created ISPN-7409 [1]. Galder, do you intend to work on this/have a specific release in mind? I noticed in the planning doc that it hasn't been marked for any release.

No, don't have any release in mind right now and I'm not currently working on it. So, feel free to look into it :)

Cheers,

>
> [1] https://issues.jboss.org/browse/ISPN-7409
>
>> I would have liked the default user type marshaller to use
>> jboss-marshalling with the all the 8.2.x internal externalizers, in
>> order to have backwards compatibility for the stores without needing
>> an upgrade tool. But it seems like the stores change their layout all
>> the time anyway, so I would settle for some version metadata in the
>> store and better error messages instead.
>
> +1 to adding some version metadata.
>
>> I'm not sure about the enum, don't like the feeling that marshallers
>> not bundled with the core are somehow second-class. I'd rather have a
>> single marshaller name string attribute, have a NAME constant in each
>> marshaller class, and register the names on startup, either via
>> ServiceLoader or via ModuleLifecycle.
>>
>> There's also the question of whether we should support passing in a
>> marshaller instance instead of a class name (I think we should, at
>> least for testing) and whether we should allow custom configuration
>> attributes/elements for the marshaller in the XML configuration (no
>> idea).
>
> All sounds good.
>
>> Cheers
>> Dan
>>
>>
>>
>> On Tue, Mar 14, 2017 at 1:26 PM, Ryan Emerson <[hidden email]> wrote:
>>> Hi All,
>>>
>>> Currently the CacheWriterInterceptor utilises the internal marshaller for
>>> marshalling entries before they are sent to the configured cache stores.
>>> This causes several problems [1], most notably that changes to the
>>> internal marshaller make stored data incompatible across Infinispan
>>> versions.
>>>
>>> I propose that we decouple the internal and store marshaller. To allow 9.x
>>> versions to remain compatible, we should default to the internal
>>> marshaller (until 10.x at least), but optionally allow users to specify a
>>> custom StreamingMarshaller implementation as part of their
>>> PersistenceConfiguration. As we already have the protostuff and kryo
>>> bridges, users would specify an enum for the marshaller they want to use
>>> as well as an optional class string if a custom implementation is
>>> required. So for example:
>>>
>>> enum StoreMarshaller {
>>>   CUSTOM,
>>>   INTERNAL,
>>>   KRYO
>>>   PROTOSTUFF;
>>> }
>>>
>>> new ConfigurationBuilder()
>>>    .persistence()
>>>        .marshaller(StoreMarshaller.CUSTOM)
>>>        .marshallerClass("org.example.Marshaller")
>>>         ...
>>>
>>> Finally, Gustavo brought flatbuffers[2] to my attention which could be a
>>> good option to provide for users as one of the default StoreMarshaller
>>> implementations.
>>>
>>> WDYT?
>>>
>>> Cheers
>>> Ryan
>>>
>>> [1]
>>> https://docs.google.com/document/d/1PR0eYgjhqXUR5w03npS7TdOs2KDZjfdMh0au8XAsyJY
>>> [2] https://google.github.io/flatbuffers/
>>> _______________________________________________
>>> 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


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