Skip to content

Commit 0d05e87

Browse files
zhichenzhichen
committed
Refactoring bulk stats test and add some java docs as mentioned in the review.
1 parent 270e8d5 commit 0d05e87

File tree

11 files changed

+95
-46
lines changed

11 files changed

+95
-46
lines changed

‎rest-api-spec/src/main/resources/rest-api-spec/api/indices.stats.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@
8484
"segments",
8585
"store",
8686
"warmer",
87-
"suggest"
87+
"suggest",
88+
"bulk"
8889
],
8990
"description":"Limit the information returned the specific metrics."
9091
}

‎rest-api-spec/src/main/resources/rest-api-spec/api/nodes.stats.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@
168168
"segments",
169169
"store",
170170
"warmer",
171-
"suggest"
171+
"suggest",
172+
"bulk"
172173
],
173174
"description":"Limit the information returned for `indices` metric to the specific index metrics. Isn't used if `indices` (or `all`) metric isn't specified."
174175
},

‎rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@
7878
warmer.current .+ \n
7979
warmer.total .+ \n
8080
warmer.total_time .+ \n
81+
bulk.total_operations .+ \n
82+
bulk.total_time .+ \n
83+
bulk.total_size_in_bytes .+ \n
8184
$/
8285
---
8386
"Test cat shards output":

‎server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ protected void doRun() throws Exception {
168168
}
169169
// We're done, there's no more operations to execute so we resolve the wrapped listener
170170
finishRequest();
171+
primary.getBulkOperationListener().afterBulk(request.totalSizeInBytes(), System.nanoTime() - startBulkTime);
171172
}
172173

173174
@Override
@@ -189,7 +190,6 @@ private void finishRequest() {
189190
() -> new WritePrimaryResult<>(
190191
context.getBulkShardRequest(), context.buildShardResponse(), context.getLocationToSync(), null,
191192
context.getPrimary(), logger));
192-
primary.getBulkOperationListener().afterBulk(request.totalSizeInBytes(), System.nanoTime() - startBulkTime);
193193
}
194194
}.run();
195195
}

‎server/src/main/java/org/elasticsearch/index/bulk/stats/BulkStats.java‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,15 @@
2828
import org.elasticsearch.common.xcontent.XContentBuilder;
2929

3030
import java.io.IOException;
31+
import java.util.Objects;
3132

33+
/**
34+
* Bulk related statistics, including the time and size of shard bulk requests,
35+
* starting at the shard level and allowing aggregation to indices and node level
36+
*/
3237
public class BulkStats implements Writeable, ToXContentFragment {
3338

34-
private long total = 0;
39+
private long totalOperations = 0;
3540
private long totalTimeInMillis = 0;
3641
private long totalSizeInBytes = 0;
3742

@@ -40,13 +45,13 @@ public BulkStats() {
4045
}
4146

4247
public BulkStats(StreamInput in) throws IOException {
43-
total = in.readVLong();
48+
totalOperations = in.readVLong();
4449
totalTimeInMillis = in.readVLong();
4550
totalSizeInBytes = in.readVLong();
4651
}
4752

48-
public BulkStats(long total, long totalTimeInMillis, long totalSizeInBytes) {
49-
this.total = total;
53+
public BulkStats(long totalOperations, long totalTimeInMillis, long totalSizeInBytes) {
54+
this.totalOperations = totalOperations;
5055
this.totalTimeInMillis = totalTimeInMillis;
5156
this.totalSizeInBytes = totalSizeInBytes;
5257
}
@@ -59,7 +64,7 @@ public void addTotals(BulkStats bulkStats) {
5964
if (bulkStats == null) {
6065
return;
6166
}
62-
this.total += bulkStats.total;
67+
this.totalOperations += bulkStats.totalOperations;
6368
this.totalTimeInMillis += bulkStats.totalTimeInMillis;
6469
this.totalSizeInBytes += bulkStats.totalSizeInBytes;
6570
}
@@ -68,8 +73,8 @@ public long getTotalSizeInBytes() {
6873
return totalSizeInBytes;
6974
}
7075

71-
public long getTotal() {
72-
return total;
76+
public long getTotalOperations() {
77+
return totalOperations;
7378
}
7479

7580
public TimeValue getTotalTime() {
@@ -80,24 +85,46 @@ public long getTotalTimeInMillis() {
8085
return totalTimeInMillis;
8186
}
8287

83-
@Override public void writeTo(StreamOutput out) throws IOException {
84-
out.writeVLong(total);
88+
@Override
89+
public void writeTo(StreamOutput out) throws IOException {
90+
out.writeVLong(totalOperations);
8591
out.writeVLong(totalTimeInMillis);
8692
out.writeVLong(totalSizeInBytes);
8793
}
8894

89-
@Override public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
95+
@Override
96+
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
9097
builder.startObject(Fields.BULK);
91-
builder.field(Fields.TOTAL, total);
98+
builder.field(Fields.TOTAL_OPERATIONS, totalOperations);
9299
builder.humanReadableField(Fields.TOTAL_TIME_IN_MILLIS, Fields.TOTAL_TIME, getTotalTime());
93100
builder.field(Fields.TOTAL_SIZE_IN_BYTES, totalSizeInBytes);
94101
builder.endObject();
95102
return builder;
96103
}
97104

105+
@Override
106+
public boolean equals(Object o) {
107+
if (this == o) {
108+
return true;
109+
}
110+
if (o == null || getClass() != o.getClass()) {
111+
return false;
112+
}
113+
114+
final BulkStats that = (BulkStats) o;
115+
return Objects.equals(this.totalOperations, that.totalOperations)
116+
&& Objects.equals(this.totalTimeInMillis, that.totalTimeInMillis)
117+
&& Objects.equals(this.totalSizeInBytes, that.totalSizeInBytes);
118+
}
119+
120+
@Override
121+
public int hashCode() {
122+
return Objects.hash(totalOperations, totalTimeInMillis, totalSizeInBytes);
123+
}
124+
98125
static final class Fields {
99126
static final String BULK = "bulk";
100-
static final String TOTAL = "total";
127+
static final String TOTAL_OPERATIONS = "total_operations";
101128
static final String TOTAL_TIME = "total_time";
102129
static final String TOTAL_TIME_IN_MILLIS = "total_time_in_millis";
103130
static final String TOTAL_SIZE_IN_BYTES = "total_size_in_bytes";

‎server/src/main/java/org/elasticsearch/index/bulk/stats/ShardBulkStats.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,14 @@
2121

2222
import org.elasticsearch.common.metrics.CounterMetric;
2323
import org.elasticsearch.common.metrics.MeanMetric;
24+
import org.elasticsearch.index.shard.IndexShard;
2425

2526
import java.util.concurrent.TimeUnit;
2627

28+
/**
29+
* Internal class that maintains relevant shard bulk statistics / metrics.
30+
* @see IndexShard
31+
*/
2732
public class ShardBulkStats implements BulkOperationListener {
2833

2934
private final StatsHolder totalStats = new StatsHolder();
@@ -32,7 +37,8 @@ public BulkStats stats() {
3237
return totalStats.stats();
3338
}
3439

35-
@Override public void afterBulk(long shardBulkSizeInBytes, long tookInNanos) {
40+
@Override
41+
public void afterBulk(long shardBulkSizeInBytes, long tookInNanos) {
3642
totalStats.totalSizeInBytes.inc(shardBulkSizeInBytes);
3743
totalStats.shardBulkMetric.inc(tookInNanos);
3844
}

‎server/src/main/java/org/elasticsearch/rest/action/cat/RestIndicesAction.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,9 @@ protected Table getTableWithHeader(final RestRequest request) {
487487

488488
table.addCell("search.throttled", "alias:sth;default:false;desc:indicates if the index is search throttled");
489489

490-
table.addCell("bulk.total",
491-
"sibling:pri;alias:bto,bulkTotal;default:false;text-align:right;desc:number of bulk shard ops");
492-
table.addCell("pri.bulk.shard_bulk_time", "default:false;text-align:right;desc:number of bulk shard ops");
490+
table.addCell("bulk.total_operations",
491+
"sibling:pri;alias:bto,bulkTotalOperation;default:false;text-align:right;desc:number of bulk shard ops");
492+
table.addCell("pri.bulk.total_operations", "default:false;text-align:right;desc:number of bulk shard ops");
493493

494494
table.addCell("bulk.total_time", "sibling:pri;alias:btti,bulkTotalTime;default:false;text-align:right;desc:time spend in shard bulk");
495495
table.addCell("pri.bulk.shard_bulk_time", "default:false;text-align:right;desc:time spend in shard bulk");
@@ -756,8 +756,8 @@ Table buildTable(final RestRequest request,
756756

757757
table.addCell(searchThrottled);
758758

759-
table.addCell(totalStats.getBulk() == null ? null : totalStats.getBulk().getTotal());
760-
table.addCell(primaryStats.getBulk() == null ? null : primaryStats.getBulk().getTotal());
759+
table.addCell(totalStats.getBulk() == null ? null : totalStats.getBulk().getTotalOperations());
760+
table.addCell(primaryStats.getBulk() == null ? null : primaryStats.getBulk().getTotalOperations());
761761

762762
table.addCell(totalStats.getBulk() == null ? null : totalStats.getBulk().getTotalTime());
763763
table.addCell(primaryStats.getBulk() == null ? null : primaryStats.getBulk().getTotalTime());

‎server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ protected Table getTableWithHeader(final RestRequest request) {
243243
table.addCell("suggest.time", "alias:suti,suggestTime;default:false;text-align:right;desc:time spend in suggest");
244244
table.addCell("suggest.total", "alias:suto,suggestTotal;default:false;text-align:right;desc:number of suggest ops");
245245

246-
table.addCell("bulk.total", "alias:bto,bulkTotal;default:false;text-align:right;desc:number of bulk shard ops");
246+
table.addCell("bulk.total_operations", "alias:bto,bulkTotalOperations;default:false;text-align:right;desc:number of bulk shard ops");
247247
table.addCell("bulk.total_time", "alias:btti,bulkTotalTime;default:false;text-align:right;desc:time spend in shard bulk");
248248
table.addCell("bulk.total_size_in_bytes", "alias:btsi,bulkTotalSizeInBytes;default:false;text-align:right;desc:total size in bytes of shard bulk");
249249

@@ -422,7 +422,7 @@ Table buildTable(boolean fullId, RestRequest req, ClusterStateResponse state, No
422422
table.addCell(searchStats == null ? null : searchStats.getTotal().getSuggestCount());
423423

424424
BulkStats bulkStats = indicesStats == null ? null : indicesStats.getBulk();
425-
table.addCell(bulkStats == null ? null : bulkStats.getTotal());
425+
table.addCell(bulkStats == null ? null : bulkStats.getTotalOperations());
426426
table.addCell(bulkStats == null ? null : bulkStats.getTotalTime());
427427
table.addCell(bulkStats == null ? null : bulkStats.getTotalSizeInBytes());
428428

‎server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ protected Table getTableWithHeader(final RestRequest request) {
200200
table.addCell("warmer.total", "alias:wto,warmerTotal;default:false;text-align:right;desc:total warmer ops");
201201
table.addCell("warmer.total_time", "alias:wtt,warmerTotalTime;default:false;text-align:right;desc:time spent in warmers");
202202

203-
table.addCell("bulk.total", "alias:bto,bulkTotal;default:false;text-align:right;desc:number of bulk shard ops");
203+
table.addCell("bulk.total_operations", "alias:bto,bulkTotalOperations;default:false;text-align:right;desc:number of bulk shard ops");
204204
table.addCell("bulk.total_time", "alias:btti,bulkTotalTime;default:false;text-align:right;desc:time spend in shard bulk");
205205
table.addCell("bulk.total_size_in_bytes", "alias:btsi,bulkTotalSizeInBytes;default:false;text-align:right;desc:total size in bytes of shard bulk");
206206

@@ -355,7 +355,7 @@ private Table buildTable(RestRequest request, ClusterStateResponse state, Indice
355355
table.addCell(getOrNull(commonStats, CommonStats::getWarmer, WarmerStats::total));
356356
table.addCell(getOrNull(commonStats, CommonStats::getWarmer, WarmerStats::totalTime));
357357

358-
table.addCell(getOrNull(commonStats, CommonStats::getBulk, BulkStats::getTotal));
358+
table.addCell(getOrNull(commonStats, CommonStats::getBulk, BulkStats::getTotalOperations));
359359
table.addCell(getOrNull(commonStats, CommonStats::getBulk, BulkStats::getTotalTime));
360360
table.addCell(getOrNull(commonStats, CommonStats::getBulk, BulkStats::getTotalSizeInBytes));
361361

‎server/src/test/java/org/elasticsearch/index/bulk/stats/BulkStatsTests.java‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,34 @@
1919

2020
package org.elasticsearch.index.bulk.stats;
2121

22-
import org.elasticsearch.common.io.stream.BytesStreamOutput;
23-
import org.elasticsearch.common.io.stream.StreamInput;
24-
import org.elasticsearch.test.ESTestCase;
25-
26-
import java.io.IOException;
27-
28-
public class BulkStatsTests extends ESTestCase {
29-
30-
public void testSerialize() throws IOException {
31-
BulkStats stats = new BulkStats(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
32-
BytesStreamOutput out = new BytesStreamOutput();
33-
stats.writeTo(out);
34-
StreamInput input = out.bytes().streamInput();
35-
BulkStats read = new BulkStats(input);
36-
assertEquals(-1, input.read());
37-
assertEquals(stats.getTotal(), read.getTotal());
38-
assertEquals(stats.getTotalTime(), read.getTotalTime());
39-
assertEquals(stats.getTotalSizeInBytes(), read.getTotalSizeInBytes());
22+
import org.elasticsearch.common.io.stream.Writeable;
23+
import org.elasticsearch.test.AbstractWireSerializingTestCase;
24+
25+
public class BulkStatsTests extends AbstractWireSerializingTestCase<BulkStats> {
26+
27+
@Override
28+
protected Writeable.Reader<BulkStats> instanceReader() {
29+
return BulkStats::new;
30+
}
31+
32+
@Override
33+
protected BulkStats createTestInstance() {
34+
return new BulkStats(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
35+
}
36+
37+
@Override
38+
protected BulkStats mutateInstance(BulkStats instance) {
39+
BulkStats mutateBulkStats = new BulkStats(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
40+
switch (between(0, 1)) {
41+
case 0:
42+
break;
43+
case 1:
44+
mutateBulkStats.add(instance);
45+
break;
46+
default:
47+
throw new AssertionError("Illegal randomisation branch");
48+
}
49+
return mutateBulkStats;
4050
}
4151

4252
public void testAddTotals() {
@@ -54,9 +64,10 @@ public void testAddTotals() {
5464
}
5565

5666
private static void assertStats(BulkStats stats, long equalTo) {
57-
assertEquals(equalTo, stats.getTotal());
67+
assertEquals(equalTo, stats.getTotalOperations());
5868
assertEquals(equalTo, stats.getTotalTimeInMillis());
5969
assertEquals(equalTo, stats.getTotalSizeInBytes());
6070
}
71+
6172
}
6273

0 commit comments

Comments
 (0)