Skip to content

Commit 94d7f22

Browse files
arteamarteam
authored
Simplify TransportStats assertions in v9 (#114700)
Transport handling times were added in #80581 (8.1), we don't need assertions for version prior to that in 9.0
1 parent 6a52675 commit 94d7f22

File tree

3 files changed

+32
-97
lines changed

3 files changed

+32
-97
lines changed

‎server/src/main/java/org/elasticsearch/TransportVersions.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ static TransportVersion def(int id) {
175175
public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED_BACKPORT_8_X = def(8_840_0_01);
176176
public static final TransportVersion ELASTICSEARCH_9_0 = def(9_000_0_00);
177177
public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED = def(9_001_0_00);
178+
public static final TransportVersion TRANSPORT_STATS_HANDLING_TIME_REQUIRED = def(9_002_0_00);
179+
178180
/*
179181
* STOP! READ THIS FIRST! No, really,
180182
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _

‎server/src/main/java/org/elasticsearch/transport/TransportStats.java‎

Lines changed: 26 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.elasticsearch.common.unit.ByteSizeValue;
1919
import org.elasticsearch.common.xcontent.ChunkedToXContent;
2020
import org.elasticsearch.core.TimeValue;
21-
import org.elasticsearch.core.UpdateForV9;
2221
import org.elasticsearch.xcontent.ToXContent;
2322
import org.elasticsearch.xcontent.XContentBuilder;
2423

@@ -70,18 +69,16 @@ public TransportStats(StreamInput in) throws IOException {
7069
rxSize = in.readVLong();
7170
txCount = in.readVLong();
7271
txSize = in.readVLong();
73-
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_1_0) && in.readBoolean()) {
74-
inboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT];
75-
for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) {
76-
inboundHandlingTimeBucketFrequencies[i] = in.readVLong();
77-
}
78-
outboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT];
79-
for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) {
80-
outboundHandlingTimeBucketFrequencies[i] = in.readVLong();
81-
}
82-
} else {
83-
inboundHandlingTimeBucketFrequencies = new long[0];
84-
outboundHandlingTimeBucketFrequencies = new long[0];
72+
if (in.getTransportVersion().before(TransportVersions.TRANSPORT_STATS_HANDLING_TIME_REQUIRED)) {
73+
in.readBoolean();
74+
}
75+
inboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT];
76+
for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) {
77+
inboundHandlingTimeBucketFrequencies[i] = in.readVLong();
78+
}
79+
outboundHandlingTimeBucketFrequencies = new long[HandlingTimeTracker.BUCKET_COUNT];
80+
for (int i = 0; i < inboundHandlingTimeBucketFrequencies.length; i++) {
81+
outboundHandlingTimeBucketFrequencies[i] = in.readVLong();
8582
}
8683
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_8_0)) {
8784
transportActionStats = Collections.unmodifiableMap(in.readOrderedMap(StreamInput::readString, TransportActionStats::new));
@@ -99,15 +96,16 @@ public void writeTo(StreamOutput out) throws IOException {
9996
out.writeVLong(rxSize);
10097
out.writeVLong(txCount);
10198
out.writeVLong(txSize);
102-
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_1_0)) {
103-
assert (inboundHandlingTimeBucketFrequencies.length > 0) == (outboundHandlingTimeBucketFrequencies.length > 0);
104-
out.writeBoolean(inboundHandlingTimeBucketFrequencies.length > 0);
105-
for (long handlingTimeBucketFrequency : inboundHandlingTimeBucketFrequencies) {
106-
out.writeVLong(handlingTimeBucketFrequency);
107-
}
108-
for (long handlingTimeBucketFrequency : outboundHandlingTimeBucketFrequencies) {
109-
out.writeVLong(handlingTimeBucketFrequency);
110-
}
99+
assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT;
100+
assert outboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT;
101+
if (out.getTransportVersion().before(TransportVersions.TRANSPORT_STATS_HANDLING_TIME_REQUIRED)) {
102+
out.writeBoolean(true);
103+
}
104+
for (long handlingTimeBucketFrequency : inboundHandlingTimeBucketFrequencies) {
105+
out.writeVLong(handlingTimeBucketFrequency);
106+
}
107+
for (long handlingTimeBucketFrequency : outboundHandlingTimeBucketFrequencies) {
108+
out.writeVLong(handlingTimeBucketFrequency);
111109
}
112110
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_8_0)) {
113111
out.writeMap(transportActionStats, StreamOutput::writeWriteable);
@@ -166,24 +164,13 @@ public Map<String, TransportActionStats> getTransportActionStats() {
166164
return transportActionStats;
167165
}
168166

169-
@UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION)
170-
// Review and simplify the if-else blocks containing this symbol once v9 is released
171-
private static final boolean IMPOSSIBLE_IN_V9 = true;
172-
173167
private boolean assertHistogramsConsistent() {
174168
assert inboundHandlingTimeBucketFrequencies.length == outboundHandlingTimeBucketFrequencies.length;
175-
if (inboundHandlingTimeBucketFrequencies.length == 0) {
176-
// Stats came from before v8.1
177-
assert IMPOSSIBLE_IN_V9;
178-
} else {
179-
assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT;
180-
}
169+
assert inboundHandlingTimeBucketFrequencies.length == HandlingTimeTracker.BUCKET_COUNT;
181170
return true;
182171
}
183172

184173
@Override
185-
@UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION)
186-
// review the "if" blocks checking for non-empty once we have
187174
public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params outerParams) {
188175
return Iterators.concat(Iterators.single((builder, params) -> {
189176
builder.startObject(Fields.TRANSPORT);
@@ -193,19 +180,10 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params outerP
193180
builder.humanReadableField(Fields.RX_SIZE_IN_BYTES, Fields.RX_SIZE, ByteSizeValue.ofBytes(rxSize));
194181
builder.field(Fields.TX_COUNT, txCount);
195182
builder.humanReadableField(Fields.TX_SIZE_IN_BYTES, Fields.TX_SIZE, ByteSizeValue.ofBytes(txSize));
196-
if (inboundHandlingTimeBucketFrequencies.length > 0) {
197-
histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM);
198-
histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM);
199-
} else {
200-
// Stats came from before v8.1
201-
assert IMPOSSIBLE_IN_V9;
202-
}
203-
if (transportActionStats.isEmpty() == false) {
204-
builder.startObject(Fields.ACTIONS);
205-
} else {
206-
// Stats came from before v8.8
207-
assert IMPOSSIBLE_IN_V9;
208-
}
183+
assert inboundHandlingTimeBucketFrequencies.length > 0;
184+
histogramToXContent(builder, inboundHandlingTimeBucketFrequencies, Fields.INBOUND_HANDLING_TIME_HISTOGRAM);
185+
histogramToXContent(builder, outboundHandlingTimeBucketFrequencies, Fields.OUTBOUND_HANDLING_TIME_HISTOGRAM);
186+
builder.startObject(Fields.ACTIONS);
209187
return builder;
210188
}),
211189

@@ -215,12 +193,7 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params outerP
215193
return builder;
216194
}),
217195

218-
Iterators.single((builder, params) -> {
219-
if (transportActionStats.isEmpty() == false) {
220-
builder.endObject();
221-
}
222-
return builder.endObject();
223-
})
196+
Iterators.single((builder, params) -> { return builder.endObject().endObject(); })
224197
);
225198
}
226199

‎server/src/test/java/org/elasticsearch/transport/TransportStatsTests.java‎

Lines changed: 4 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,50 +20,8 @@
2020

2121
public class TransportStatsTests extends ESTestCase {
2222
public void testToXContent() {
23-
assertEquals(
24-
Strings.toString(
25-
new TransportStats(1, 2, 3, ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), new long[0], new long[0], Map.of()),
26-
false,
27-
true
28-
),
29-
"""
30-
{"transport":{"server_open":1,"total_outbound_connections":2,\
31-
"rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\
32-
"tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456\
33-
}}"""
34-
);
35-
3623
final var histogram = new long[HandlingTimeTracker.BUCKET_COUNT];
37-
assertEquals(
38-
Strings.toString(
39-
new TransportStats(1, 2, 3, ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), histogram, histogram, Map.of()),
40-
false,
41-
true
42-
),
43-
"""
44-
{"transport":{"server_open":1,"total_outbound_connections":2,\
45-
"rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\
46-
"tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456,\
47-
"inbound_handling_time_histogram":[],\
48-
"outbound_handling_time_histogram":[]\
49-
}}"""
50-
);
51-
5224
histogram[4] = 10;
53-
assertEquals(
54-
Strings.toString(
55-
new TransportStats(1, 2, 3, ByteSizeUnit.MB.toBytes(4), 5, ByteSizeUnit.MB.toBytes(6), histogram, histogram, Map.of()),
56-
false,
57-
true
58-
),
59-
"""
60-
{"transport":{"server_open":1,"total_outbound_connections":2,\
61-
"rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\
62-
"tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456,\
63-
"inbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}],\
64-
"outbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}]\
65-
}}"""
66-
);
6725

6826
final var requestSizeHistogram = new long[29];
6927
requestSizeHistogram[2] = 9;
@@ -84,8 +42,8 @@ public void testToXContent() {
8442
ByteSizeUnit.MB.toBytes(4),
8543
5,
8644
ByteSizeUnit.MB.toBytes(6),
87-
new long[0],
88-
new long[0],
45+
histogram,
46+
histogram,
8947
Map.of("internal:test/action", exampleActionStats)
9048
),
9149
false,
@@ -95,6 +53,8 @@ public void testToXContent() {
9553
{"transport":{"server_open":1,"total_outbound_connections":2,\
9654
"rx_count":3,"rx_size":"4mb","rx_size_in_bytes":4194304,\
9755
"tx_count":5,"tx_size":"6mb","tx_size_in_bytes":6291456,\
56+
"inbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}],\
57+
"outbound_handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}],\
9858
"actions":{"internal:test/action":%s}}}""", Strings.toString(exampleActionStats, false, true))
9959
);
10060
}

0 commit comments

Comments
 (0)