Skip to content

Commit d7e9e09

Browse files
authored
Add Builder in IndexingStats and refactor usage (#19306)
Signed-off-by: Jean Kim <[email protected]>
1 parent 1173bdf commit d7e9e09

File tree

4 files changed

+177
-35
lines changed

4 files changed

+177
-35
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2323
- Remove MultiCollectorWrapper and use MultiCollector in Lucene instead ([#19595](https://github.com/opensearch-project/OpenSearch/pull/19595))
2424
- Change implementation for `percentiles` aggregation for latency improvement ([#19648](https://github.com/opensearch-project/OpenSearch/pull/19648))
2525
- Refactor the ThreadPoolStats.Stats class to use the Builder pattern instead of constructors ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
26+
- Refactor the IndexingStats.Stats class to use the Builder pattern instead of constructors ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
2627

2728
### Fixed
2829
- Fix Allocation and Rebalance Constraints of WeightFunction are incorrectly reset ([#19012](https://github.com/opensearch-project/OpenSearch/pull/19012))
@@ -47,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4748

4849
### Deprecated
4950
- Deprecated existing constructors in ThreadPoolStats.Stats in favor of the new Builder ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
51+
- Deprecated existing constructors in IndexingStats.Stats in favor of the new Builder ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
5052

5153
### Removed
5254

server/src/main/java/org/opensearch/index/shard/IndexingStats.java

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,26 @@ public void writeTo(StreamOutput out) throws IOException {
165165
docStatusStats = new DocStatusStats();
166166
}
167167

168+
/**
169+
* Private constructor that takes a builder.
170+
* This is the sole entry point for creating a new Stats object.
171+
* @param builder The builder instance containing all the values.
172+
*/
173+
private Stats(Builder builder) {
174+
this.indexCount = builder.indexCount;
175+
this.indexTimeInMillis = builder.indexTimeInMillis;
176+
this.indexCurrent = builder.indexCurrent;
177+
this.indexFailedCount = builder.indexFailedCount;
178+
this.deleteCount = builder.deleteCount;
179+
this.deleteTimeInMillis = builder.deleteTimeInMillis;
180+
this.deleteCurrent = builder.deleteCurrent;
181+
this.noopUpdateCount = builder.noopUpdateCount;
182+
this.throttleTimeInMillis = builder.throttleTimeInMillis;
183+
this.isThrottled = builder.isThrottled;
184+
this.docStatusStats = builder.docStatusStats;
185+
this.maxLastIndexRequestTimestamp = builder.maxLastIndexRequestTimestamp;
186+
}
187+
168188
public Stats(StreamInput in) throws IOException {
169189
indexCount = in.readVLong();
170190
indexTimeInMillis = in.readVLong();
@@ -188,6 +208,11 @@ public Stats(StreamInput in) throws IOException {
188208
}
189209
}
190210

211+
/**
212+
* This constructor will be deprecated starting in version 3.3.0.
213+
* Use {@link Builder} instead.
214+
*/
215+
@Deprecated
191216
public Stats(
192217
long indexCount,
193218
long indexTimeInMillis,
@@ -217,6 +242,11 @@ public Stats(
217242
);
218243
}
219244

245+
/**
246+
* This constructor will be deprecated starting in version 3.3.0.
247+
* Use {@link Builder} instead.
248+
*/
249+
@Deprecated
220250
public Stats(
221251
long indexCount,
222252
long indexTimeInMillis,
@@ -386,6 +416,94 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
386416
return builder;
387417
}
388418

419+
/**
420+
* Builder for the {@link Stats} class.
421+
* Provides a fluent API for constructing a Stats object.
422+
*/
423+
public static class Builder {
424+
private long indexCount = 0;
425+
private long indexTimeInMillis = 0;
426+
private long indexCurrent = 0;
427+
private long indexFailedCount = 0;
428+
private long deleteCount = 0;
429+
private long deleteTimeInMillis = 0;
430+
private long deleteCurrent = 0;
431+
private long noopUpdateCount = 0;
432+
private long throttleTimeInMillis = 0;
433+
private boolean isThrottled = false;
434+
private DocStatusStats docStatusStats = null;
435+
private long maxLastIndexRequestTimestamp = 0;
436+
437+
public Builder() {}
438+
439+
public Builder indexCount(long count) {
440+
this.indexCount = count;
441+
return this;
442+
}
443+
444+
public Builder indexTimeInMillis(long time) {
445+
this.indexTimeInMillis = time;
446+
return this;
447+
}
448+
449+
public Builder indexCurrent(long current) {
450+
this.indexCurrent = current;
451+
return this;
452+
}
453+
454+
public Builder indexFailedCount(long count) {
455+
this.indexFailedCount = count;
456+
return this;
457+
}
458+
459+
public Builder deleteCount(long count) {
460+
this.deleteCount = count;
461+
return this;
462+
}
463+
464+
public Builder deleteTimeInMillis(long time) {
465+
this.deleteTimeInMillis = time;
466+
return this;
467+
}
468+
469+
public Builder deleteCurrent(long current) {
470+
this.deleteCurrent = current;
471+
return this;
472+
}
473+
474+
public Builder noopUpdateCount(long count) {
475+
this.noopUpdateCount = count;
476+
return this;
477+
}
478+
479+
public Builder throttleTimeInMillis(long time) {
480+
this.throttleTimeInMillis = time;
481+
return this;
482+
}
483+
484+
public Builder isThrottled(boolean throttled) {
485+
this.isThrottled = throttled;
486+
return this;
487+
}
488+
489+
public Builder docStatusStats(DocStatusStats stats) {
490+
this.docStatusStats = stats;
491+
return this;
492+
}
493+
494+
public Builder maxLastIndexRequestTimestamp(long timestamp) {
495+
this.maxLastIndexRequestTimestamp = timestamp;
496+
return this;
497+
}
498+
499+
/**
500+
* Creates a {@link Stats} object from the builder's current state.
501+
* @return A new Stats instance.
502+
*/
503+
public Stats build() {
504+
return new Stats(this);
505+
}
506+
}
389507
}
390508

391509
private final Stats totalStats;

server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -154,20 +154,19 @@ static class StatsHolder {
154154
private final MaxMetric maxLastIndexRequestTimestamp = new MaxMetric();
155155

156156
IndexingStats.Stats stats(boolean isThrottled, long currentThrottleMillis) {
157-
return new IndexingStats.Stats(
158-
indexMetric.count(),
159-
TimeUnit.NANOSECONDS.toMillis(indexMetric.sum()),
160-
indexCurrent.count(),
161-
indexFailed.count(),
162-
deleteMetric.count(),
163-
TimeUnit.NANOSECONDS.toMillis(deleteMetric.sum()),
164-
deleteCurrent.count(),
165-
noopUpdates.count(),
166-
isThrottled,
167-
TimeUnit.MILLISECONDS.toMillis(currentThrottleMillis),
168-
new IndexingStats.Stats.DocStatusStats(),
169-
maxLastIndexRequestTimestamp.get()
170-
);
157+
return new IndexingStats.Stats.Builder().indexCount(indexMetric.count())
158+
.indexTimeInMillis(TimeUnit.NANOSECONDS.toMillis(indexMetric.sum()))
159+
.indexCurrent(indexCurrent.count())
160+
.indexFailedCount(indexFailed.count())
161+
.deleteCount(deleteMetric.count())
162+
.deleteTimeInMillis(TimeUnit.NANOSECONDS.toMillis(deleteMetric.sum()))
163+
.deleteCurrent(deleteCurrent.count())
164+
.noopUpdateCount(noopUpdates.count())
165+
.isThrottled(isThrottled)
166+
.throttleTimeInMillis(TimeUnit.MILLISECONDS.toMillis(currentThrottleMillis))
167+
.docStatusStats(new IndexingStats.Stats.DocStatusStats())
168+
.maxLastIndexRequestTimestamp(maxLastIndexRequestTimestamp.get())
169+
.build();
171170
}
172171
}
173172
}

server/src/test/java/org/opensearch/index/shard/IndexingStatsTests.java

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,20 @@ public void testMaxLastIndexRequestTimestampAggregation() throws Exception {
128128
long ts1 = randomLongBetween(0, 1000000);
129129
long ts2 = randomLongBetween(0, 1000000);
130130
long ts3 = randomLongBetween(0, 1000000);
131-
IndexingStats.Stats stats1 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts1);
132-
IndexingStats.Stats stats2 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts2);
133-
IndexingStats.Stats stats3 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts3);
131+
IndexingStats.Stats.Builder defaultStats = new IndexingStats.Stats.Builder().indexCount(1)
132+
.indexTimeInMillis(2)
133+
.indexCurrent(3)
134+
.indexFailedCount(4)
135+
.deleteCount(5)
136+
.deleteTimeInMillis(6)
137+
.deleteCurrent(7)
138+
.noopUpdateCount(8)
139+
.isThrottled(false)
140+
.throttleTimeInMillis(9)
141+
.docStatusStats(docStatusStats);
142+
IndexingStats.Stats stats1 = defaultStats.maxLastIndexRequestTimestamp(ts1).build();
143+
IndexingStats.Stats stats2 = defaultStats.maxLastIndexRequestTimestamp(ts2).build();
144+
IndexingStats.Stats stats3 = defaultStats.maxLastIndexRequestTimestamp(ts3).build();
134145

135146
// Aggregate stats1 + stats2
136147
stats1.add(stats2);
@@ -141,20 +152,33 @@ public void testMaxLastIndexRequestTimestampAggregation() throws Exception {
141152
assertEquals(Math.max(Math.max(ts1, ts2), ts3), stats1.getMaxLastIndexRequestTimestamp());
142153

143154
// Test with zero and negative values
144-
IndexingStats.Stats statsZero = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, 0L);
145-
IndexingStats.Stats statsNeg = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, -100L);
155+
IndexingStats.Stats statsZero = defaultStats.maxLastIndexRequestTimestamp(0L).build();
156+
IndexingStats.Stats statsNeg = defaultStats.maxLastIndexRequestTimestamp(-100L).build();
157+
146158
statsZero.add(statsNeg);
147159
assertEquals(0L, statsZero.getMaxLastIndexRequestTimestamp());
148160

149-
IndexingStats.Stats statsNeg2 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, -50L);
161+
IndexingStats.Stats statsNeg2 = defaultStats.maxLastIndexRequestTimestamp(-50L).build();
150162
statsNeg.add(statsNeg2);
151163
assertEquals(-50L, statsNeg.getMaxLastIndexRequestTimestamp());
152164
}
153165

154166
public void testMaxLastIndexRequestTimestampBackwardCompatibility() throws IOException {
155167
IndexingStats.Stats.DocStatusStats docStatusStats = new IndexingStats.Stats.DocStatusStats();
156168
long ts = randomLongBetween(0, 1000000);
157-
IndexingStats.Stats stats = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts);
169+
IndexingStats.Stats stats = new IndexingStats.Stats.Builder().indexCount(1)
170+
.indexTimeInMillis(2)
171+
.indexCurrent(3)
172+
.indexFailedCount(4)
173+
.deleteCount(5)
174+
.deleteTimeInMillis(6)
175+
.deleteCurrent(7)
176+
.noopUpdateCount(8)
177+
.isThrottled(false)
178+
.throttleTimeInMillis(9)
179+
.docStatusStats(docStatusStats)
180+
.maxLastIndexRequestTimestamp(ts)
181+
.build();
158182

159183
// Serialize with V_3_1_0 (should include the field)
160184
BytesStreamOutput outNew = new BytesStreamOutput();
@@ -181,20 +205,19 @@ private IndexingStats createTestInstance() {
181205
docStatusStats.add(RestStatus.fromCode(i * 100), randomNonNegativeLong());
182206
}
183207

184-
IndexingStats.Stats stats = new IndexingStats.Stats(
185-
randomNonNegativeLong(),
186-
randomNonNegativeLong(),
187-
randomNonNegativeLong(),
188-
randomNonNegativeLong(),
189-
randomNonNegativeLong(),
190-
randomNonNegativeLong(),
191-
randomNonNegativeLong(),
192-
randomNonNegativeLong(),
193-
randomBoolean(),
194-
randomNonNegativeLong(),
195-
docStatusStats,
196-
randomLong()
197-
);
208+
IndexingStats.Stats stats = new IndexingStats.Stats.Builder().indexCount(randomNonNegativeLong())
209+
.indexTimeInMillis(randomNonNegativeLong())
210+
.indexCurrent(randomNonNegativeLong())
211+
.indexFailedCount(randomNonNegativeLong())
212+
.deleteCount(randomNonNegativeLong())
213+
.deleteTimeInMillis(randomNonNegativeLong())
214+
.deleteCurrent(randomNonNegativeLong())
215+
.noopUpdateCount(randomNonNegativeLong())
216+
.isThrottled(randomBoolean())
217+
.throttleTimeInMillis(randomNonNegativeLong())
218+
.docStatusStats(docStatusStats)
219+
.maxLastIndexRequestTimestamp(randomLong())
220+
.build();
198221

199222
return new IndexingStats(stats);
200223
}

0 commit comments

Comments
 (0)