Simplify TransportStats assertions in v9#114700
Conversation
Transport handling times were added in elastic#80581 (8.1), we don't need assertions for version prior to that in 9.0
|
Pinging @elastic/es-distributed (Team:Distributed) |
…nto v9-transport-stats-assertions
DaveCTurner
left a comment
There was a problem hiding this comment.
I would prefer that we clean up the de/serialization code at this point too.
This reverts commit 35a2901.
|
Pinging @elastic/es-distributed-obsolete (Team:Distributed (Obsolete)) |
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
929dda9 to
bb59b3b
Compare
DaveCTurner
left a comment
There was a problem hiding this comment.
I think there's room for further simplification here, or at least better alignment between the serialization and deserialization code - see comment inline.
| assert (inboundHandlingTimeBucketFrequencies.length > 0) == (outboundHandlingTimeBucketFrequencies.length > 0); | ||
| assert (inboundHandlingTimeBucketFrequencies.length > 0) == (outboundHandlingTimeBucketFrequencies.length > 0); | ||
| if (out.getTransportVersion().before(TransportVersions.TRANSPORT_STATS_HANDLING_TIME_REQUIRED)) { | ||
| out.writeBoolean(inboundHandlingTimeBucketFrequencies.length > 0); |
There was a problem hiding this comment.
This doesn't match up with the reading code which requires these things to have length HandlingTimeTracker.BUCKET_COUNT.
There was a problem hiding this comment.
I've hardened up the assertions on the bucket frequencies length in efc71fd
DaveCTurner
left a comment
There was a problem hiding this comment.
LGTM
Again, pretty important to backport this to v9.0
💔 Backport failed
You can use sqren/backport to manually backport by running |
Backports elastic#114700 to 9.0 > Transport handling times were added in elastic#80581 (8.1), we don't need assertions for version prior to that in 9.0
Transport handling times were added in #80581 (8.1), we don't need assertions for version prior to that in 9.0