7:56 a.m.
I'm having an interesting "chat" with Galder on github after a pull
request:
https://github.com/galderz/infinispan/commit/74b065d4f520d4a22dfa6ef78ae8...
The pull request itself was resolved, but we ended up brainstorming
about how we should encode some objects (Flags in this case) to make
sure we don't make it highly unlikely to be able to support rolling
upgrades in the future.. which is something we really have to think
about today imho.
Cheers,
Sanne
10:37 a.m.
Quickly tried this and caused no issues:
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
On May 9, 2012, at 2:56 PM, Sanne Grinovero wrote:
I'm having an interesting "chat" with Galder on github
after a pull request:
https://github.com/galderz/infinispan/commit/74b065d4f520d4a22dfa6ef78ae8...
The pull request itself was resolved, but we ended up brainstorming
about how we should encode some objects (Flags in this case) to make
sure we don't make it highly unlikely to be able to support rolling
upgrades in the future.. which is something we really have to think
about today imho.
Cheers,
Sanne
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
11:23 a.m.
On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
Quickly tried this and caused no issues:
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
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).
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.
On May 9, 2012, at 2:56 PM, Sanne Grinovero wrote:
> I'm having an interesting "chat" with Galder on github after a pull
request:
>
https://github.com/galderz/infinispan/commit/74b065d4f520d4a22dfa6ef78ae8...
>
> The pull request itself was resolved, but we ended up brainstorming
> about how we should encode some objects (Flags in this case) to make
> sure we don't make it highly unlikely to be able to support rolling
> upgrades in the future.. which is something we really have to think
> about today imho.
>
> Cheers,
> Sanne
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
4:30 p.m.
On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com>
wrote:
> Quickly tried this and caused no issues:
>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
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.
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/j...
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.
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
6:11 a.m.
On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>> Quickly tried this and caused no issues:
>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>
> 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.
> 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/j...
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'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.
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
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
3:36 a.m.
On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com>
wrote:
> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>> Quickly tried this and caused no issues:
>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>
>> 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/j...
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
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
5:11 a.m.
On 15 May 2012 09:36, Dan Berindei <dan.berindei(a)gmail.com> wrote:
On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
> On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>> Quickly tried this and caused no issues:
>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>
>>> 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...
I know I proposed this myself, but I would rather avoid this path..
even if some flags are unsafe, they might be safe in the configuration
/ use case, and we might want to force it to proceed nevertheless. I
think we should always have the option to "force it to proceed".
In worst case one might want to block all transactions and kill the
clients, but you have to propose a way to migrate the data.
#On two bitsets being sent..
First problem is that we're sending redundant information; second is
that it should not be responsibility of the older node (in terms of
version of the code being run) to properly understand the newer nodes,
it should be the other way around.
So rather than sending two bitsets, in this case the responsibility
belongs to the *sender*: since it's the one containing the newer code,
it might know how to deal with older nodes, or might be patched/fixed
to know how to deal with older nodes. It's obviously harder to patch
existing nodes! Especially if they are running already (and you don't
have Byteman).
If we could keep track of the version of other nodes in the View, we
could have a translation table plugged in on how to talk to a
different version node.. maintaining such a table would be much
simpler as it would live independently from the main code, and be
fixed and hacked on.. possibly even loaded dynamically. So that's what
I would do in the long term.. in the short term, let's just save some
bytes using optimal encoding for the EnumSet.
Especially of note, if the sender (newer node) knows by looking into
the translaction table that an operation/command can't be possibly
sent to the older node, it should be easier to find an alternative or
workaround.
when migrating an existing cluster by a rolling upgrade, for sure the
existing application is NOT using features which exist only in the
never version.. that's granted and we can take advantage of that.
>
>>> 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/j...
>
> 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>.
Can't we write it using the special-purpose Externalizer explicitly?
To read it, it will have it's own Externalizer ID.
>> 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.
Right, I only asked for the UNKNOWN for the time being, as we don't
have anything better yet.
And I guess it's safe to have such a flag for future.. you never know.
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
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
9:26 a.m.
Interesting discussion for sure. So what have we done in this regard? I see (a) an
optimisation for marshalling collections of Flags, and the ability to mark unknown flags
accordingly (and log this rather than crash).
On 15 May 2012, at 11:11, Sanne Grinovero wrote:
On 15 May 2012 09:36, Dan Berindei <dan.berindei(a)gmail.com>
wrote:
> On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
>> On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>>> Quickly tried this and caused no issues:
>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>
>>>> 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...
I know I proposed this myself, but I would rather avoid this path..
even if some flags are unsafe, they might be safe in the configuration
/ use case, and we might want to force it to proceed nevertheless. I
think we should always have the option to "force it to proceed".
In worst case one might want to block all transactions and kill the
clients, but you have to propose a way to migrate the data.
#On two bitsets being sent..
First problem is that we're sending redundant information; second is
that it should not be responsibility of the older node (in terms of
version of the code being run) to properly understand the newer nodes,
it should be the other way around.
So rather than sending two bitsets, in this case the responsibility
belongs to the *sender*: since it's the one containing the newer code,
it might know how to deal with older nodes, or might be patched/fixed
to know how to deal with older nodes. It's obviously harder to patch
existing nodes! Especially if they are running already (and you don't
have Byteman).
If we could keep track of the version of other nodes in the View, we
could have a translation table plugged in on how to talk to a
different version node.. maintaining such a table would be much
simpler as it would live independently from the main code, and be
fixed and hacked on.. possibly even loaded dynamically. So that's what
I would do in the long term.. in the short term, let's just save some
bytes using optimal encoding for the EnumSet.
Especially of note, if the sender (newer node) knows by looking into
the translaction table that an operation/command can't be possibly
sent to the older node, it should be easier to find an alternative or
workaround.
when migrating an existing cluster by a rolling upgrade, for sure the
existing application is NOT using features which exist only in the
never version.. that's granted and we can take advantage of that.
>
>>
>>>> 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/j...
>>
>> 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>.
Can't we write it using the special-purpose Externalizer explicitly?
To read it, it will have it's own Externalizer ID.
>>> 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.
Right, I only asked for the UNKNOWN for the time being, as we don't
have anything better yet.
And I guess it's safe to have such a flag for future.. you never know.
> 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
>>> infinispan-dev(a)lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Manik Surtani
manik(a)jboss.org
twitter.com/maniksurtani
Lead, Infinispan
http://www.infinispan.org
9:36 a.m.
On 15 May 2012 15:26, Manik Surtani <manik(a)jboss.org> wrote:
Interesting discussion for sure. So what have we done in this
regard? I see (a) an optimisation for marshalling collections of Flags, and the ability
to mark unknown flags accordingly (and log this rather than crash).
Exactly.
And as Galder suggested: https://issues.jboss.org/browse/ISPN-2044
thanks!
Sanne
On 15 May 2012, at 11:11, Sanne Grinovero wrote:
> On 15 May 2012 09:36, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>> On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>> On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
>>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com>
wrote:
>>>>>> Quickly tried this and caused no issues:
>>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>>
>>>>> 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...
>
> I know I proposed this myself, but I would rather avoid this path..
> even if some flags are unsafe, they might be safe in the configuration
> / use case, and we might want to force it to proceed nevertheless. I
> think we should always have the option to "force it to proceed".
> In worst case one might want to block all transactions and kill the
> clients, but you have to propose a way to migrate the data.
>
> #On two bitsets being sent..
>
> First problem is that we're sending redundant information; second is
> that it should not be responsibility of the older node (in terms of
> version of the code being run) to properly understand the newer nodes,
> it should be the other way around.
>
> So rather than sending two bitsets, in this case the responsibility
> belongs to the *sender*: since it's the one containing the newer code,
> it might know how to deal with older nodes, or might be patched/fixed
> to know how to deal with older nodes. It's obviously harder to patch
> existing nodes! Especially if they are running already (and you don't
> have Byteman).
> If we could keep track of the version of other nodes in the View, we
> could have a translation table plugged in on how to talk to a
> different version node.. maintaining such a table would be much
> simpler as it would live independently from the main code, and be
> fixed and hacked on.. possibly even loaded dynamically. So that's what
> I would do in the long term.. in the short term, let's just save some
> bytes using optimal encoding for the EnumSet.
>
> Especially of note, if the sender (newer node) knows by looking into
> the translaction table that an operation/command can't be possibly
> sent to the older node, it should be easier to find an alternative or
> workaround.
> when migrating an existing cluster by a rolling upgrade, for sure the
> existing application is NOT using features which exist only in the
> never version.. that's granted and we can take advantage of that.
>
>>
>>>
>>>>> 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/j...
>>>
>>> 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>.
>
> Can't we write it using the special-purpose Externalizer explicitly?
> To read it, it will have it's own Externalizer ID.
>
>
>>>> 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.
>
> Right, I only asked for the UNKNOWN for the time being, as we don't
> have anything better yet.
> And I guess it's safe to have such a flag for future.. you never know.
>
>> 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
>>>> infinispan-dev(a)lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev(a)lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Manik Surtani
manik(a)jboss.org
twitter.com/maniksurtani
Lead, Infinispan
http://www.infinispan.org
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
2:39 p.m.
On Tue, May 15, 2012 at 1:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 15 May 2012 09:36, Dan Berindei <dan.berindei(a)gmail.com>
wrote:
> On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
>> On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>>> Quickly tried this and caused no issues:
>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>
>>>> 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...
I know I proposed this myself, but I would rather avoid this path..
even if some flags are unsafe, they might be safe in the configuration
/ use case, and we might want to force it to proceed nevertheless. I
think we should always have the option to "force it to proceed".
In worst case one might want to block all transactions and kill the
clients, but you have to propose a way to migrate the data.
I'm starting to warm up to the idea of making all flags "migration
safe" myself... in the end, it's the user's decision, we just have to
make sure that he knows what to expect.
#On two bitsets being sent..
First problem is that we're sending redundant information; second is
that it should not be responsibility of the older node (in terms of
version of the code being run) to properly understand the newer nodes,
it should be the other way around.
So rather than sending two bitsets, in this case the responsibility
belongs to the *sender*: since it's the one containing the newer code,
it might know how to deal with older nodes, or might be patched/fixed
to know how to deal with older nodes. It's obviously harder to patch
existing nodes! Especially if they are running already (and you don't
have Byteman).
If we could keep track of the version of other nodes in the View, we
could have a translation table plugged in on how to talk to a
different version node.. maintaining such a table would be much
simpler as it would live independently from the main code, and be
fixed and hacked on.. possibly even loaded dynamically. So that's what
I would do in the long term.. in the short term, let's just save some
bytes using optimal encoding for the EnumSet.
Keeping track of the versions on each node and sending different
things is certainly doable, but I fear in replicated mode it would
force us to replace broadcasts with multiple unicasts. So there is a
small risk that it will bring the cluster down anyway due to extra
traffic ;)
The extra traffic during the upgrade is probably acceptable, I'm
actually more concerned about the extra checks that we'd have to do
all the time to ensure that we are not doing an upgrade at the moment
and we don't need the special treatment...
Especially of note, if the sender (newer node) knows by looking into
the translaction table that an operation/command can't be possibly
sent to the older node, it should be easier to find an alternative or
workaround.
when migrating an existing cluster by a rolling upgrade, for sure the
existing application is NOT using features which exist only in the
never version.. that's granted and we can take advantage of that.
>
>>
>>>> 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/j...
>>
>> 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>.
Can't we write it using the special-purpose Externalizer explicitly?
To read it, it will have it's own Externalizer ID.
When writing, the marshaller can only use the object's class to find
the appropriate externalizer for an object, and I'm pretty sure
getClass() will return the same thing for EnumSet<Flag> and
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.
Right, I only asked for the UNKNOWN for the time being, as we don't
have anything better yet.
And I guess it's safe to have such a flag for future.. you never know.
> 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
>>>
2:58 p.m.
On 15 May 2012 20:39, Dan Berindei <dan.berindei(a)gmail.com> wrote:
On Tue, May 15, 2012 at 1:11 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
> On 15 May 2012 09:36, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>> On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>> On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
>>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com>
wrote:
>>>>>> Quickly tried this and caused no issues:
>>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>>
>>>>> 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...
>
> I know I proposed this myself, but I would rather avoid this path..
> even if some flags are unsafe, they might be safe in the configuration
> / use case, and we might want to force it to proceed nevertheless. I
> think we should always have the option to "force it to proceed".
> In worst case one might want to block all transactions and kill the
> clients, but you have to propose a way to migrate the data.
>
I'm starting to warm up to the idea of making all flags "migration
safe" myself... in the end, it's the user's decision, we just have to
make sure that he knows what to expect.
Exactly.
> #On two bitsets being sent..
>
> First problem is that we're sending redundant information; second is
> that it should not be responsibility of the older node (in terms of
> version of the code being run) to properly understand the newer nodes,
> it should be the other way around.
>
> So rather than sending two bitsets, in this case the responsibility
> belongs to the *sender*: since it's the one containing the newer code,
> it might know how to deal with older nodes, or might be patched/fixed
> to know how to deal with older nodes. It's obviously harder to patch
> existing nodes! Especially if they are running already (and you don't
> have Byteman).
> If we could keep track of the version of other nodes in the View, we
> could have a translation table plugged in on how to talk to a
> different version node.. maintaining such a table would be much
> simpler as it would live independently from the main code, and be
> fixed and hacked on.. possibly even loaded dynamically. So that's what
> I would do in the long term.. in the short term, let's just save some
> bytes using optimal encoding for the EnumSet.
>
Keeping track of the versions on each node and sending different
things is certainly doable, but I fear in replicated mode it would
force us to replace broadcasts with multiple unicasts. So there is a
small risk that it will bring the cluster down anyway due to extra
traffic ;)
Good point. That's the good thing that we discuss these things now, we
don't have to implement it all right away but make sure that current
versions have enough code in it to make a future migration possible.
So to improve odds on this, we could either use the JGroups feature
which actually sends multicast but discards the packet on certain
nodes (don't remember the name), or think already to have Infinispan
discard all packets it receives which have an header higher than it's
known version.
We should do this only if we make such a translation feature,
otherwise we're locking ourself out from any further rolling upgrade
plan ;-)
The extra traffic during the upgrade is probably acceptable, I'm
actually more concerned about the extra checks that we'd have to do
all the time to ensure that we are not doing an upgrade at the moment
and we don't need the special treatment...
This special behaviour could be enabled by a configuration option. We
could migrate from 5.2 to 6.0-hybrid and then from 6.0-hybrid to 6.0,
where 6.0-hybrid and 6.0 are the same codebase, just that the hybrid
is having a "retro" translation capability activated.
> Especially of note, if the sender (newer node) knows by looking into
> the translaction table that an operation/command can't be possibly
> sent to the older node, it should be easier to find an alternative or
> workaround.
> when migrating an existing cluster by a rolling upgrade, for sure the
> existing application is NOT using features which exist only in the
> never version.. that's granted and we can take advantage of that.
>
>>
>>>
>>>>> 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/j...
>>>
>>> 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>.
>
> Can't we write it using the special-purpose Externalizer explicitly?
> To read it, it will have it's own Externalizer ID.
>
When writing, the marshaller can only use the object's class to find
the appropriate externalizer for an object, and I'm pretty sure
getClass() will return the same thing for EnumSet<Flag> and
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.
>
> Right, I only asked for the UNKNOWN for the time being, as we don't
> have anything better yet.
> And I guess it's safe to have such a flag for future.. you never know.
>
>> 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
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
5:21 a.m.
On May 15, 2012, at 10:36 AM, Dan Berindei wrote:
On Mon, May 14, 2012 at 2:11 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
> On 11 May 2012 22:30, Dan Berindei <dan.berindei(a)gmail.com> wrote:
>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>> Quickly tried this and caused no issues:
>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>
>>> 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…
Hmmmm, not sure..
>
>>> 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/j...
>
> 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>.
-1 to encoding class name. You don't even know it at runtime cos of lack of reified
generics.
Easiest is to wrap EnumSet<Flag> around a custom type and register it in
ExternalizerTable.
>> 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.
Fine
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.
Why should the receiver reject the command? It might work in non-optimal way, or ignore
certain stuff, but I don't see a case where it should reject.
Rejecting would in fact stop normal rolling upgrades.
>> 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
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
2:44 a.m.
On May 11, 2012, at 11:30 PM, Dan Berindei wrote:
On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>> Quickly tried this and caused no issues:
>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>
> 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.
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.
> 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/j...
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.
Cheers
Dan
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
3:52 a.m.
On Tue, May 15, 2012 at 10:44 AM, Galder Zamarreño <galder(a)redhat.com> wrote:
On May 11, 2012, at 11:30 PM, Dan Berindei wrote:
> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>> Quickly tried this and caused no issues:
>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>
>> 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.
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".
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/j...
>
> 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.
The HotRod flags are serialized directly in Codec10.writeHeader, which
is the equivalent of ReplicableCommandExternalizer.writeCommandHeader.
5:24 a.m.
On May 15, 2012, at 10:52 AM, Dan Berindei wrote:
On Tue, May 15, 2012 at 10:44 AM, Galder Zamarreño
<galder(a)redhat.com> wrote:
>
> On May 11, 2012, at 11:30 PM, Dan Berindei wrote:
>
>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>> Quickly tried this and caused no issues:
>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>
>>> 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/j...
>>
>> 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.
The HotRod flags are serialized directly in Codec10.writeHeader, which
is the equivalent of ReplicableCommandExternalizer.writeCommandHeader.
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
5:34 a.m.
On 16 May 2012 11:24, Galder Zamarreño <galder(a)redhat.com> wrote:
On May 15, 2012, at 10:52 AM, Dan Berindei wrote:
> On Tue, May 15, 2012 at 10:44 AM, Galder Zamarreño <galder(a)redhat.com> wrote:
>>
>> On May 11, 2012, at 11:30 PM, Dan Berindei wrote:
>>
>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>>> Quickly tried this and caused no issues:
>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>
>>>> 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/j...
>>>
>>> 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
> infinispan-dev(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
10:45 a.m.
On Wed, May 16, 2012 at 1:24 PM, Galder Zamarreño <galder(a)redhat.com> wrote:
On May 15, 2012, at 10:52 AM, Dan Berindei wrote:
> On Tue, May 15, 2012 at 10:44 AM, Galder Zamarreño <galder(a)redhat.com> wrote:
>>
>> On May 11, 2012, at 11:30 PM, Dan Berindei wrote:
>>
>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com> wrote:
>>>>> Quickly tried this and caused no issues:
>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>
>>>> 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.
Well, we already have 5.0.1.Final, and I'm pretty sure it's
incompatible with 5.0.0.Final :)
But yes, I agree that we could release a version that handles a new
flag correctly yet never sends it to other nodes.
> 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 think there are cases where upgrading nodes one by one (a "true"
rolling upgrade) does not make sense, because the Infinispan versions
are just too different to work together in the same cluster.
But there are probably many other places where we'd be blowing up in
flames in such cases, so it's not worth adding complications to the
flags serialization format just for that.
> 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/j...
>>>
>>> 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.
I think I prefer writing the flags explicitly in the
ReplicableCommandExternalizer, to avoid creating lots of extra object
instances.
>
> The HotRod flags are serialized directly in Codec10.writeHeader, which
> is the equivalent of ReplicableCommandExternalizer.writeCommandHeader.
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
2:35 a.m.
On May 16, 2012, at 5:45 PM, Dan Berindei wrote:
On Wed, May 16, 2012 at 1:24 PM, Galder Zamarreño
<galder(a)redhat.com> wrote:
>
>
> On May 15, 2012, at 10:52 AM, Dan Berindei wrote:
>
>> On Tue, May 15, 2012 at 10:44 AM, Galder Zamarreño <galder(a)redhat.com>
wrote:
>>>
>>> On May 11, 2012, at 11:30 PM, Dan Berindei wrote:
>>>
>>>> On Fri, May 11, 2012 at 7:23 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
>>>>> On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com>
wrote:
>>>>>> Quickly tried this and caused no issues:
>>>>>>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
>>>>>
>>>>> 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.
>
Well, we already have 5.0.1.Final, and I'm pretty sure it's
incompatible with 5.0.0.Final :)
But yes, I agree that we could release a version that handles a new
flag correctly yet never sends it to other nodes.
>> 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 think there are cases where upgrading nodes one by one (a "true"
rolling upgrade) does not make sense, because the Infinispan versions
are just too different to work together in the same cluster.
But there are probably many other places where we'd be blowing up in
flames in such cases, so it's not worth adding complications to the
flags serialization format just for that.
>
>> 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/j...
>>>>
>>>> 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.
>
I think I prefer writing the flags explicitly in the
ReplicableCommandExternalizer, to avoid creating lots of extra object
instances.
^ That could work. I added a note to ISPN-2044. Thanks
>
>>
>> The HotRod flags are serialized directly in Codec10.writeHeader, which
>> is the equivalent of ReplicableCommandExternalizer.writeCommandHeader.
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
2:37 a.m.
On May 11, 2012, at 6:23 PM, Sanne Grinovero wrote:
On 11 May 2012 16:37, Galder Zamarreño <galder(a)redhat.com>
wrote:
> Quickly tried this and caused no issues:
>
https://github.com/galderz/infinispan/commit/7718926e5a4a6763506250362d7b...
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).
^ I agree. I'm working on https://issues.jboss.org/browse/ISPN-2034 which should
highligh this kind of issues.
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.
Makes sense. We already do a similar thing for Hot Rod flags. Can you add a jira for it
and assign it to me?
>
> On May 9, 2012, at 2:56 PM, Sanne Grinovero wrote:
>
>> I'm having an interesting "chat" with Galder on github after a pull
request:
>>
https://github.com/galderz/infinispan/commit/74b065d4f520d4a22dfa6ef78ae8...
>>
>> The pull request itself was resolved, but we ended up brainstorming
>> about how we should encode some objects (Flags in this case) to make
>> sure we don't make it highly unlikely to be able to support rolling
>> upgrades in the future.. which is something we really have to think
>> about today imho.
>>
>> Cheers,
>> Sanne
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
4381
days inactive
4390
days old
infinispan-dev@lists.jboss.org
18 comments
4 participants
participants (4)
-
Dan Berindei -
Galder Zamarreño -
Manik Surtani -
Sanne Grinovero