Fix Decision.Type serialization BWC#140199
Conversation
nicktindall
added
>bug
:Distributed/Allocation
elasticsearchmachine
added
v9.4.0
Team:Distributed Coordination (obsolete)
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
|
Hi @nicktindall, I've created a changelog YAML for you. |
nicktindall
added
v9.3.1
auto-backport
|
|
||
| public void testSerializationBackwardCompatibility() throws IOException { | ||
| testReadWriteEnum(YES, YES, ALLOCATION_DECISION_NOT_PREFERRED); | ||
| testReadWriteEnum(NOT_PREFERRED, THROTTLE, ALLOCATION_DECISION_NOT_PREFERRED); |
There was a problem hiding this comment.
I find it a bit confusing and even a slight disturbing to test NOT_PREFERRED is written/read as THROTTLE when the versions do not match. What we really want to ensure is the same enum is preserved across the wire regardless of versions, except NOT_PREFERRED which can be YES for very old versions. Is it possible to more directly test the high level outcome and skip the implementation details?
There was a problem hiding this comment.
Refactored in 45a9d5b to be a bit clearer
|
9.3.0 has not yet released. I think we should raise this and the other PR as a blocker. |
|
If we do release this PR and #140197 as part of 9.3.0, they can be labelled as |
| List.of(NO, THROTTLE).forEach(d -> assertFalse(d.assignmentAllowed())); | ||
| } | ||
|
|
||
| public void testSerializationBackwardCompatibility() throws IOException { |
There was a problem hiding this comment.
Can we also test the opposite, i.e. an old enum is written by an old node and deserialized by a new node? Or do you think it's sufficiently covered by the roundTripValue assertions? It's quite similar but the writing side still uses the latest code with conditional branching. I wonder how it compares to writing with the old-style enums unconidtionally?
There was a problem hiding this comment.
I think it is equivalent to what's there by virtue of the fact we assert the old ordinals are written then that they're interpreted correctly when read back by the current enum?
|
@elasticsearchmachine test this please |
💚 Backport successful
|
4 participants
Decision.Typeorder was changed in #137228, but no corresponding change was made serialize it in a BWC manner.This PR just implements backwardly-compatible serialization for
Decision.Type