On 16 May 2012 11:24, Galder Zamarreño <[email protected]> wrote: > > On May 15, 2012, at 10:52 AM, Dan Berindei wrote: > >> On Tue, May 15, 2012 at 10:44 AM, Galder Zamarreño <[email protected]> wrote: >>> >>> On May 11, 2012, at 11:30 PM, Dan Berindei wrote: >>> >>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <[email protected]> >>>> wrote: >>>>> On 11 May 2012 16:37, Galder Zamarreño <[email protected]> wrote: >>>>>> Quickly tried this and caused no issues: >>>>>> https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7bd5cbdccd2931 >>>>> >>>>> Looks good! I'm sure this doesn't solve all future migration problems, >>>>> but if we could keep this kind of tricks around it should improve >>>>> odds. >>>>> IMHO, this is a kind of sensitivity that we should apply across all >>>>> areas (not just flags). >>>>> >>>> >>>> Looks interesting, but then you have the opposite problem: not all new >>>> flags can be ignored, so you need a way to specify that a new flag is >>>> "required". E.g. if we had just added a ZERO_LOCK_ACQUISITION_TIMEOUT >>>> flag then the client would be expecting spurious failures, but not >>>> extra long delays. >>> >>> Hmmm, I disagree. If you're adding a new flag, say in 5.2, and you expect a >>> node that runs 5.0 to deal with it properly, really, what you need to be >>> doing is implementing that flag in 5.0. >>> >> >> Well, 5.0 is already out there, so modifying it is not an option. > > You can always release a 5.0.x :), and I'm pretty sure we might have to do > some micro releases to make rolling upgrades work. > >> What we can do is ensure that the clients see the incompatibility in >> their testing environment and don't use two versions in production >> without being aware of the problem. >> >>> We want: >>> - if an old client encounters a new/unknown marshalled value, to not blow >>> up and log a WARN. >>> - if an old client is expected to react to a to a new/unknown marshalled >>> value in the way new versions deal with it, it'll need to implement it. >>> >>> We don't want: >>> - old client to 'blow up in flames' when they encounter new/unknown >>> options, since this causes problems with potential rolling upgrades. >>> >> >> I think there are situations where two versions really are >> incompatible and we really should "blow up in flames". > > Any blowing up in flames would stop rolling upgrades from working, so find me > a real example of this first and try to understand how rolling upgrade would > work in that scenario... > >> I'm not saying that's justified in all cases or even in the majority >> of cases, but I'm pretty sure it's not going to be 0% either. >> >>>> >>>>> On a totally different page, why are we serializing Flags one-by-one ? >>>>> We mostly need to serialize EnumSets right? >>>>> An EnumSet can be encoded by using the bits of a couple of bytes. >>>>> Three bytes looks like enough for all our needs.. we could even be >>>>> clever and reserve a special Externalizer-ID for the empty set, to >>>>> avoid 3 bytes where none are needed. >>>>> While currently we need an integer (4 bytes) to encode the header for >>>>> "EnumSet", plus (4 bytes header + 1 byte value) * each flag -> a lot. >>>>> >>>> >>>> RiverMarshaller already has an optimization for the empty set: >>>> https://github.com/dmlloyd/jboss-marshalling/blob/master/river/src/main/java/org/jboss/marshalling/river/RiverMarshaller.java#L613 >>>> >>>> I'm not sure why it doesn't encode each element as a bit, it might be >>>> to keep wire compatibility when the order of values in an enum >>>> changes. >>> >>> David? >>> >>>> However, because there is only one EnumSet for all Enum types, a >>>> hypothetical EnumSetExternalizer also needs to write the name of the >>>> enum class - if we wanted to serialize EnumSet<Flag> in 2 bytes then >>>> we'd need to make the transformation in ReplicableCommandExternalizer. >>> >>> Not necessarily. I think we should do what Sanne suggests but manually in >>> the Flag.Externalizer class, since that's tied to the Flag type. >>> >>> Within it, we can replicate what an enum set does for marshalling. We >>> already have such code in the Hot Rod server/client (that's how we handle >>> flags there - completely forgot about it when I wrote Flag.Externalizer), >>> so shouldn't be a biggie. >>> >> >> The call stack looks like this: ReplicableCommandExternalizer -> >> EnumSet externalizer (in RiverMarshaller) -> Flag.Externalizer. >> You can't change how the EnumSet is serialized in Flag.Externalizer, >> you have to modify either the EnumSet externalizer (e.g. by writing a >> new FlagSet class) or ReplicableCommandExternalizer. > > Writing a new FlagSet class is my fav. Easy to do and easy to resgister in > the ext table.
+1 @Dan I had some related comments on https://issues.jboss.org/browse/ISPN-2044 .. let's continue discussion of minor details there? > >> >> The HotRod flags are serialized directly in Codec10.writeHeader, which >> is the equivalent of ReplicableCommandExternalizer.writeCommandHeader. >> >> _______________________________________________ >> infinispan-dev mailing list >> [email protected] >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Galder Zamarreño > Sr. Software Engineer > Infinispan, JBoss Cache > > > _______________________________________________ > infinispan-dev mailing list > [email protected] > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
