From 00f255dc074f7752ac4f31b7f3222a01f6c66815 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 14 Oct 2024 10:49:38 +0200 Subject: [PATCH 1/9] Simplfy TransportStats assertions in v9 Transport handling times were added in #80581 (8.1), we don't need assertions for version prior to that in 9.0 --- .../transport/TransportStats.java | 30 +++---------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index 46b161b01e9f3..9203b9e2322d9 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.xcontent.ChunkedToXContent; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; @@ -166,24 +165,13 @@ public Map getTransportActionStats() { return transportActionStats; } - @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) - // Review and simplify the if-else blocks containing this symbol once v9 is released - private static final boolean IMPOSSIBLE_IN_V9 = true; - private boolean assertHistogramsConsistent() { assert inboundHandlingTimeBucketFrequencies.length == outboundHandlingTimeBucketFrequencies.length; - if (inboundHandlingTimeBucketFrequencies.length == 0) { - // Stats came from before v8.1 - assert IMPOSSIBLE_IN_V9; - } else { - assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; - } + assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; return true; } @Override - @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) - // review the "if" blocks checking for non-empty once we have public Iterator toXContentChunked(ToXContent.Params outerParams) { return Iterators.concat(Iterators.single((builder, params) -> { builder.startObject(Fields.TRANSPORT); @@ -193,19 +181,9 @@ public Iterator toXContentChunked(ToXContent.Params outerP builder.humanReadableField(Fields.RX_SIZE_IN_BYTES, Fields.RX_SIZE, ByteSizeValue.ofBytes(rxSize)); builder.field(Fields.TX_COUNT, txCount); builder.humanReadableField(Fields.TX_SIZE_IN_BYTES, Fields.TX_SIZE, ByteSizeValue.ofBytes(txSize)); - if (inboundHandlingTimeBucketFrequencies.length > 0) { - histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); - histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); - } else { - // Stats came from before v8.1 - assert IMPOSSIBLE_IN_V9; - } - if (transportActionStats.isEmpty() == false) { - builder.startObject(Fields.ACTIONS); - } else { - // Stats came from before v8.8 - assert IMPOSSIBLE_IN_V9; - } + histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); + histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); + builder.startObject(Fields.ACTIONS); return builder; }), From 55fa9224d6f6a9ac1ad503b389f1f8736e09b3a7 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Wed, 16 Oct 2024 11:37:26 +0200 Subject: [PATCH 2/9] Keep condition checks for transport stats assertions in v9 --- .../org/elasticsearch/transport/TransportStats.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index 9203b9e2322d9..be8c7249c81d5 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -167,7 +167,8 @@ public Map getTransportActionStats() { private boolean assertHistogramsConsistent() { assert inboundHandlingTimeBucketFrequencies.length == outboundHandlingTimeBucketFrequencies.length; - assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; + assert inboundHandlingTimeBucketFrequencies.length == 0 + || inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; return true; } @@ -181,9 +182,13 @@ public Iterator toXContentChunked(ToXContent.Params outerP builder.humanReadableField(Fields.RX_SIZE_IN_BYTES, Fields.RX_SIZE, ByteSizeValue.ofBytes(rxSize)); builder.field(Fields.TX_COUNT, txCount); builder.humanReadableField(Fields.TX_SIZE_IN_BYTES, Fields.TX_SIZE, ByteSizeValue.ofBytes(txSize)); - histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); - histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); - builder.startObject(Fields.ACTIONS); + if (inboundHandlingTimeBucketFrequencies.length > 0) { + histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); + histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); + } + if (transportActionStats.isEmpty() == false) { + builder.startObject(Fields.ACTIONS); + } return builder; }), From 578ccae5b40fb2ba81aa8a4b555c6daef9c26b14 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Thu, 24 Oct 2024 00:07:20 +0200 Subject: [PATCH 3/9] Simplify assertions --- .../transport/TransportStats.java | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index be8c7249c81d5..8ea8b969ce214 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -182,13 +182,10 @@ public Iterator toXContentChunked(ToXContent.Params outerP builder.humanReadableField(Fields.RX_SIZE_IN_BYTES, Fields.RX_SIZE, ByteSizeValue.ofBytes(rxSize)); builder.field(Fields.TX_COUNT, txCount); builder.humanReadableField(Fields.TX_SIZE_IN_BYTES, Fields.TX_SIZE, ByteSizeValue.ofBytes(txSize)); - if (inboundHandlingTimeBucketFrequencies.length > 0) { - histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); - histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); - } - if (transportActionStats.isEmpty() == false) { - builder.startObject(Fields.ACTIONS); - } + assert inboundHandlingTimeBucketFrequencies.length > 0; + histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); + histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); + builder.startObject(Fields.ACTIONS); return builder; }), @@ -198,12 +195,7 @@ public Iterator toXContentChunked(ToXContent.Params outerP return builder; }), - Iterators.single((builder, params) -> { - if (transportActionStats.isEmpty() == false) { - builder.endObject(); - } - return builder.endObject(); - }) + Iterators.single((builder, params) -> { return builder.endObject().endObject(); }) ); } From 45f47f3aba596c5283bf46f3ef2c659a7b6f30df Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Thu, 24 Oct 2024 00:15:35 +0200 Subject: [PATCH 4/9] Test transport stats in 9.0 format --- .../transport/TransportStatsTests.java | 48 ++----------------- 1 file changed, 4 insertions(+), 44 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/transport/TransportStatsTests.java b/server/src/test/java/org/elasticsearch/transport/TransportStatsTests.java index c3965547abb5d..1c9cb4c9afc0f 100644 --- a/server/src/test/java/org/elasticsearch/transport/TransportStatsTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TransportStatsTests.java @@ -20,50 +20,8 @@ public class TransportStatsTests extends ESTestCase { public void testToXContent() { - assertEquals( - Strings.toString( - new TransportStats(1, 2, 3, ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), new long[0], new long[0], Map.of()), - false, - true - ), - """ - {"transport":{"server_open":1,"total_outbound_connections":2,\ - "rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\ - "tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456\ - }}""" - ); - final var histogram = new long[HandlingTimeTracker.BUCKET_COUNT]; - assertEquals( - Strings.toString( - new TransportStats(1, 2, 3, ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), histogram, histogram, Map.of()), - false, - true - ), - """ - {"transport":{"server_open":1,"total_outbound_connections":2,\ - "rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\ - "tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456,\ - "inbound_handling_time_histogram":[],\ - "outbound_handling_time_histogram":[]\ - }}""" - ); - histogram[4] = 10; - assertEquals( - Strings.toString( - new TransportStats(1, 2, 3, ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), histogram, histogram, Map.of()), - false, - true - ), - """ - {"transport":{"server_open":1,"total_outbound_connections":2,\ - "rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\ - "tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456,\ - "inbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}],\ - "outbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}]\ - }}""" - ); final var requestSizeHistogram = new long[29]; requestSizeHistogram[2] = 9; @@ -84,8 +42,8 @@ public void testToXContent() { ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), - new long[0], - new long[0], + histogram, + histogram, Map.of("internal:test/action", exampleActionStats) ), false, @@ -95,6 +53,8 @@ public void testToXContent() { {"transport":{"server_open":1,"total_outbound_connections":2,\ "rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\ "tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456,\ + "inbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}],\ + "outbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}],\ "actions":{"internal:test/action":%s}}}""", Strings.toString(exampleActionStats, false, true)) ); } From 237f6e7c42c2990a5aba4b474f806b13e53ac25d Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Thu, 24 Oct 2024 11:46:04 +0200 Subject: [PATCH 5/9] Simplify consistency checks --- .../main/java/org/elasticsearch/transport/TransportStats.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index 8ea8b969ce214..bae6566e06bc0 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -167,8 +167,7 @@ public Map getTransportActionStats() { private boolean assertHistogramsConsistent() { assert inboundHandlingTimeBucketFrequencies.length == outboundHandlingTimeBucketFrequencies.length; - assert inboundHandlingTimeBucketFrequencies.length == 0 - || inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; + assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; return true; } From 35a290104715e2030603a3ceda394679a6f9de6f Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 4 Nov 2024 09:35:13 +0100 Subject: [PATCH 6/9] Assert transportActionStats is not empty --- .../main/java/org/elasticsearch/transport/TransportStats.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index bae6566e06bc0..22a7b902a0a26 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -184,6 +184,7 @@ public Iterator toXContentChunked(ToXContent.Params outerP assert inboundHandlingTimeBucketFrequencies.length > 0; histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); + assert transportActionStats.isEmpty() == false; builder.startObject(Fields.ACTIONS); return builder; }), From d15016e657fba75bcc0521961bd00b66aa1e9899 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 4 Nov 2024 10:14:21 +0100 Subject: [PATCH 7/9] Revert "Assert transportActionStats is not empty" This reverts commit 35a290104715e2030603a3ceda394679a6f9de6f. --- .../main/java/org/elasticsearch/transport/TransportStats.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index 22a7b902a0a26..bae6566e06bc0 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -184,7 +184,6 @@ public Iterator toXContentChunked(ToXContent.Params outerP assert inboundHandlingTimeBucketFrequencies.length > 0; histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM); histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM); - assert transportActionStats.isEmpty() == false; builder.startObject(Fields.ACTIONS); return builder; }), From bb59b3bda62144a425ce5a3be16c0e723a7b8640 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Mon, 3 Feb 2025 19:32:18 +0100 Subject: [PATCH 8/9] Remove version 8_1 checks --- .../org/elasticsearch/TransportVersions.java | 1 + .../transport/TransportStats.java | 38 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index efcebbec31c92..28b5872d99c7a 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -173,6 +173,7 @@ static TransportVersion def(int id) { public static final TransportVersion INFERENCE_REQUEST_ADAPTIVE_RATE_LIMITING = def(8_839_0_00); public static final TransportVersion ML_INFERENCE_IBM_WATSONX_RERANK_ADDED = def(8_840_0_00); public static final TransportVersion ELASTICSEARCH_9_0 = def(9_000_0_00); + public static final TransportVersion TRANSPORT_STATS_HANDLING_TIME_REQUIRED = def(9_0002_0_00); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index bae6566e06bc0..4f12d499b01f2 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -69,18 +69,16 @@ public TransportStats(StreamInput in) throws IOException { rxSize = in.readVLong(); txCount = in.readVLong(); txSize = in.readVLong(); - if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_1_0) && in.readBoolean()) { - inboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT]; - for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) { - inboundHandlingTimeBucketFrequencies[i] = in.readVLong(); - } - outboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT]; - for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) { - outboundHandlingTimeBucketFrequencies[i] = in.readVLong(); - } - } else { - inboundHandlingTimeBucketFrequencies = new long[0]; - outboundHandlingTimeBucketFrequencies = new long[0]; + if (in.getTransportVersion().before(TransportVersions.TRANSPORT_STATS_HANDLING_TIME_REQUIRED)) { + in.readBoolean(); + } + inboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT]; + for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) { + inboundHandlingTimeBucketFrequencies[i] = in.readVLong(); + } + outboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT]; + for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) { + outboundHandlingTimeBucketFrequencies[i] = in.readVLong(); } if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_8_0)) { transportActionStats = Collections.unmodifiableMap(in.readOrderedMap(StreamInput::readString, TransportActionStats::new)); @@ -98,15 +96,15 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLong(rxSize); out.writeVLong(txCount); out.writeVLong(txSize); - if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_1_0)) { - 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); - for (long handlingTimeBucketFrequency : inboundHandlingTimeBucketFrequencies) { - out.writeVLong(handlingTimeBucketFrequency); - } - for (long handlingTimeBucketFrequency : outboundHandlingTimeBucketFrequencies) { - out.writeVLong(handlingTimeBucketFrequency); - } + } + for (long handlingTimeBucketFrequency : inboundHandlingTimeBucketFrequencies) { + out.writeVLong(handlingTimeBucketFrequency); + } + for (long handlingTimeBucketFrequency : outboundHandlingTimeBucketFrequencies) { + out.writeVLong(handlingTimeBucketFrequency); } if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_8_0)) { out.writeMap(transportActionStats, StreamOutput::writeWriteable); From efc71fd0334535a74845b31adec0c20a7773b316 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Wed, 5 Feb 2025 12:30:53 +0100 Subject: [PATCH 9/9] Ensure that inbound and outbound bucket should have the length of BUCKET_COUNT --- .../java/org/elasticsearch/transport/TransportStats.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportStats.java b/server/src/main/java/org/elasticsearch/transport/TransportStats.java index 4f12d499b01f2..1163cfbcb2708 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportStats.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportStats.java @@ -96,9 +96,10 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLong(rxSize); out.writeVLong(txCount); out.writeVLong(txSize); - assert (inboundHandlingTimeBucketFrequencies.length > 0) == (outboundHandlingTimeBucketFrequencies.length > 0); + assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; + assert outboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT; if (out.getTransportVersion().before(TransportVersions.TRANSPORT_STATS_HANDLING_TIME_REQUIRED)) { - out.writeBoolean(inboundHandlingTimeBucketFrequencies.length > 0); + out.writeBoolean(true); } for (long handlingTimeBucketFrequency : inboundHandlingTimeBucketFrequencies) { out.writeVLong(handlingTimeBucketFrequency);