On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero <[email protected]> wrote: > On 11 May 2012 22:30, Dan Berindei <[email protected]> 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. > > You're right - but did you read the conversation on github? We already > pointed this out, still I believe we should have an option to ignore > unknown flags if/when/exclusively we think the migration is safe: we > should be able to tell after the fact, possibly even write migration > tests, but can't predict the future. > > We could also use a single bit in the externalized representation of a > flag to mean "safe to be ignored" for any flag, but I'm not sure that > all cases would be black/white .. more likely it will depend on use > case or actual configuration. >
Right, the flag serialized format should tell the receiver whether it can be ignored. But if we encode an EnumSet<Flag> as a bitset, each flag will have only one bit, so we wouldn't have a place for the "migration safe" bit. I guess we could send two bit sets, one with the flags and one with the "migration safe" bits... > >>> 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 > > That code makes perfectly sense in a general purpose use case, but it > still needs to serialize the Class definition: we can avoid that, so > we should! > I think we need to encode the Flag class name as long as we consider the flag set as just another parameter in the Object[] parameter array - so the externalizer doesn't know how to distinguish between an EnumSet<Flag> and an EnumSet<UserEnumType>. >> 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. > > That's a safe behaviour, expected for a default use case. But if we > decide to add the UNKNOWN flag, we could use bitsets. > I think the UNKNOWN flag is useful as long as we parse each flag independently, but if we start encoding the flags as a bitset (or two), then we don't need UNKNOWN any more. We would only need to ensure the position of a flag in the bitset remains constant between versions, which means either relying on the declaration order and keeping that constant (no removing or reordering of flags) or adding a position field to the enum. Assuming the order in the bitset remains constant from one version to the next, there are only two scenarios: a) The added flags are not important, and the receiver can just ignore them - in which case the parsed flag set will only include the flags that the receiver knows about. b) The added flags are important, and the receiver should reject the command - in which case the externalizer should just throw an exception instead of reading the flag set and setting the UNKNOWN flag. >> 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. >> >> Cheers >> Dan >> >> _______________________________________________ >> 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 _______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
