Skip to content

Commit dc9c1b3

Browse files
pado0pado0
authored
Refactor DocStats and StoreStats with Builder pattern (#19863)
* Add Builder in DocStats and refactor usage Signed-off-by: Jean Kim <bgshhd95@gmail.com> * Add Builder in StoreStats and refactor usage Signed-off-by: Jean Kim <bgshhd95@gmail.com> * Add Change Log Signed-off-by: Jean Kim <bgshhd95@gmail.com> --------- Signed-off-by: Jean Kim <bgshhd95@gmail.com>
1 parent d822f62 commit dc9c1b3

File tree

15 files changed

+242
-73
lines changed

15 files changed

+242
-73
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3434
- Change the default value of doc_values in WildcardFieldMapper to true. ([#19796](https://github.com/opensearch-project/OpenSearch/pull/19796))
3535
- Make Engine#loadHistoryUUID() protected and Origin#isFromTranslog() public ([#19753](https://github.com/opensearch-project/OpenSearch/pull/19752))
3636
- Bump opensearch-protobufs dependency to 0.23.0 and update transport-grpc module compatibility ([#19831](https://github.com/opensearch-project/OpenSearch/pull/19831))
37+
- Refactor the DocStats and StoreStats class to use the Builder pattern instead of constructors ([#19863](https://github.com/opensearch-project/OpenSearch/pull/19863))
3738

3839
### Fixed
3940
- Fix Allocation and Rebalance Constraints of WeightFunction are incorrectly reset ([#19012](https://github.com/opensearch-project/OpenSearch/pull/19012))
@@ -74,6 +75,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
7475
### Deprecated
7576
- Deprecated existing constructors in ThreadPoolStats.Stats in favor of the new Builder ([#19317](https://github.com/opensearch-project/OpenSearch/pull/19317))
7677
- Deprecated existing constructors in IndexingStats.Stats in favor of the new Builder ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306))
78+
- Deprecated existing constructors in DocStats and StoreStats in favor of the new Builder ([#19863](https://github.com/opensearch-project/OpenSearch/pull/19863))
7779

7880
### Removed
7981

‎modules/store-subdirectory/src/test/java/org/opensearch/plugin/store/subdirectory/SubdirectoryStorePluginTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public void testStats() throws IOException {
8787

8888
final long otherStatsBytes = randomLongBetween(0L, Integer.MAX_VALUE);
8989
final long otherStatsReservedBytes = randomBoolean() ? StoreStats.UNKNOWN_RESERVED_BYTES : randomLongBetween(0L, Integer.MAX_VALUE);
90-
stats.add(new StoreStats(otherStatsBytes, otherStatsReservedBytes));
90+
stats.add(new StoreStats.Builder().sizeInBytes(otherStatsBytes).reservedSize(otherStatsReservedBytes).build());
9191
assertEquals(initialStoreSize + otherStatsBytes, stats.getSize().getBytes());
9292
assertEquals(Math.max(reservedBytes, 0L) + Math.max(otherStatsReservedBytes, 0L), stats.getReservedSize().getBytes());
9393

‎server/src/main/java/org/opensearch/index/engine/Engine.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ protected final DocsStats docsStats(IndexReader indexReader) {
275275
logger.trace(() -> new ParameterizedMessage("failed to get size for [{}]", info.info.name), e);
276276
}
277277
}
278-
return new DocsStats(numDocs, numDeletedDocs, sizeInBytes);
278+
return new DocsStats.Builder().count(numDocs).deleted(numDeletedDocs).totalSizeInBytes(sizeInBytes).build();
279279
}
280280

281281
/**

‎server/src/main/java/org/opensearch/index/shard/DocsStats.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,28 @@ public DocsStats() {
5858

5959
}
6060

61+
/**
62+
* Private constructor that takes a builder.
63+
* This is the sole entry point for creating a new DocsStats object.
64+
* @param builder The builder instance containing all the values.
65+
*/
66+
private DocsStats(Builder builder) {
67+
this.count = builder.count;
68+
this.deleted = builder.deleted;
69+
this.totalSizeInBytes = builder.totalSizeInBytes;
70+
}
71+
6172
public DocsStats(StreamInput in) throws IOException {
6273
count = in.readVLong();
6374
deleted = in.readVLong();
6475
totalSizeInBytes = in.readVLong();
6576
}
6677

78+
/**
79+
* This constructor will be deprecated starting in version 3.4.0.
80+
* Use {@link Builder} instead.
81+
*/
82+
@Deprecated
6783
public DocsStats(long count, long deleted, long totalSizeInBytes) {
6884
this.count = count;
6985
this.deleted = deleted;
@@ -107,6 +123,41 @@ public long getAverageSizeInBytes() {
107123
return totalDocs == 0 ? 0 : totalSizeInBytes / totalDocs;
108124
}
109125

126+
/**
127+
* Builder for the {@link DocsStats} class.
128+
* Provides a fluent API for constructing a DocsStats object.
129+
*/
130+
public static class Builder {
131+
private long count = 0;
132+
private long deleted = 0;
133+
private long totalSizeInBytes = 0;
134+
135+
public Builder() {}
136+
137+
public Builder count(long count) {
138+
this.count = count;
139+
return this;
140+
}
141+
142+
public Builder deleted(long deleted) {
143+
this.deleted = deleted;
144+
return this;
145+
}
146+
147+
public Builder totalSizeInBytes(long totalSizeInBytes) {
148+
this.totalSizeInBytes = totalSizeInBytes;
149+
return this;
150+
}
151+
152+
/**
153+
* Creates a {@link DocsStats} object from the builder's current state.
154+
* @return A new DocsStats instance.
155+
*/
156+
public DocsStats build() {
157+
return new DocsStats(this);
158+
}
159+
}
160+
110161
@Override
111162
public void writeTo(StreamOutput out) throws IOException {
112163
out.writeVLong(count);

‎server/src/main/java/org/opensearch/index/store/Store.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public CheckIndex.Status checkIndex(PrintStream out) throws IOException {
469469
*/
470470
public StoreStats stats(long reservedBytes) throws IOException {
471471
ensureOpen();
472-
return new StoreStats(directory.estimateSize(), reservedBytes);
472+
return new StoreStats.Builder().sizeInBytes(directory.estimateSize()).reservedSize(reservedBytes).build();
473473
}
474474

475475
/**

‎server/src/main/java/org/opensearch/index/store/StoreStats.java‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,26 @@ public StoreStats() {
6262

6363
}
6464

65+
/**
66+
* Private constructor that takes a builder.
67+
* This is the sole entry point for creating a new StoreStats object.
68+
* @param builder The builder instance containing all the values.
69+
*/
70+
private StoreStats(Builder builder) {
71+
this.sizeInBytes = builder.sizeInBytes;
72+
this.reservedSize = builder.reservedSize;
73+
}
74+
6575
public StoreStats(StreamInput in) throws IOException {
6676
sizeInBytes = in.readVLong();
6777
reservedSize = in.readZLong();
6878
}
6979

7080
/**
71-
* @param sizeInBytes the size of the store in bytes
72-
* @param reservedSize a prediction of how much larger the store is expected to grow, or {@link StoreStats#UNKNOWN_RESERVED_BYTES}.
73-
*/
81+
* This constructor will be deprecated starting in version 3.4.0.
82+
* Use {@link Builder} instead.
83+
*/
84+
@Deprecated
7485
public StoreStats(long sizeInBytes, long reservedSize) {
7586
assert reservedSize == UNKNOWN_RESERVED_BYTES || reservedSize >= 0 : reservedSize;
7687
this.sizeInBytes = sizeInBytes;
@@ -114,6 +125,42 @@ public ByteSizeValue getReservedSize() {
114125
return new ByteSizeValue(reservedSize);
115126
}
116127

128+
/**
129+
* Builder for the {@link StoreStats} class.
130+
* Provides a fluent API for constructing a StoreStats object.
131+
*/
132+
public static class Builder {
133+
private long sizeInBytes = 0;
134+
private long reservedSize = 0;
135+
136+
public Builder() {}
137+
138+
/**
139+
* @param bytes the size of the store in bytes
140+
*/
141+
public Builder sizeInBytes(long bytes) {
142+
this.sizeInBytes = bytes;
143+
return this;
144+
}
145+
146+
/**
147+
* @param size a prediction of how much larger the store is expected to grow, or {@link StoreStats#UNKNOWN_RESERVED_BYTES}.
148+
*/
149+
public Builder reservedSize(long size) {
150+
assert size == UNKNOWN_RESERVED_BYTES || size >= 0 : size;
151+
this.reservedSize = size;
152+
return this;
153+
}
154+
155+
/**
156+
* Creates a {@link StoreStats} object from the builder's current state.
157+
* @return A new StoreStats instance.
158+
*/
159+
public StoreStats build() {
160+
return new StoreStats(this);
161+
}
162+
}
163+
117164
@Override
118165
public void writeTo(StreamOutput out) throws IOException {
119166
out.writeVLong(sizeInBytes);

‎server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,10 +1147,10 @@ public void testOldVersionNodes() throws IOException {
11471147
long numDeletedDocs = randomLongBetween(0, 100);
11481148
CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE);
11491149

1150-
commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0);
1151-
commonStats.store = new StoreStats(100, 0L);
1150+
commonStats.docs = new DocsStats.Builder().count(numDocs).deleted(numDeletedDocs).totalSizeInBytes(0).build();
1151+
commonStats.store = new StoreStats.Builder().sizeInBytes(100).reservedSize(0L).build();
11521152
commonStats.indexing = new IndexingStats();
1153-
DocsStats hostDocStats = new DocsStats(numDocs, numDeletedDocs, 0);
1153+
DocsStats hostDocStats = new DocsStats.Builder().count(numDocs).deleted(numDeletedDocs).totalSizeInBytes(0).build();
11541154

11551155
CommonStatsFlags commonStatsFlags = new CommonStatsFlags();
11561156
commonStatsFlags.clear();
@@ -1193,8 +1193,8 @@ public void testNodeIndicesStatsSerialization() throws IOException {
11931193
levelParams.add(NodeIndicesStats.StatsLevel.NODE);
11941194
CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE);
11951195

1196-
commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0);
1197-
commonStats.store = new StoreStats(100, 0L);
1196+
commonStats.docs = new DocsStats.Builder().count(numDocs).deleted(numDeletedDocs).totalSizeInBytes(0).build();
1197+
commonStats.store = new StoreStats.Builder().sizeInBytes(100).reservedSize(0L).build();
11981198
commonStats.indexing = new IndexingStats();
11991199

12001200
CommonStatsFlags commonStatsFlags = new CommonStatsFlags();
@@ -1244,8 +1244,8 @@ public void testNodeIndicesStatsToXContent() {
12441244
levelParams.add(NodeIndicesStats.StatsLevel.NODE);
12451245
CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE);
12461246

1247-
commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0);
1248-
commonStats.store = new StoreStats(100, 0L);
1247+
commonStats.docs = new DocsStats.Builder().count(numDocs).deleted(numDeletedDocs).totalSizeInBytes(0).build();
1248+
commonStats.store = new StoreStats.Builder().sizeInBytes(100).reservedSize(0L).build();
12491249
commonStats.indexing = new IndexingStats();
12501250

12511251
CommonStatsFlags commonStatsFlags = new CommonStatsFlags();
@@ -1364,8 +1364,13 @@ public void testNodeIndicesStatsWithAndWithoutAggregations() throws IOException
13641364

13651365
private CommonStats createRandomCommonStats() {
13661366
CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE);
1367-
commonStats.docs = new DocsStats(randomLongBetween(0, 10000), randomLongBetween(0, 100), randomLongBetween(0, 1000));
1368-
commonStats.store = new StoreStats(randomLongBetween(0, 100), randomLongBetween(0, 1000));
1367+
commonStats.docs = new DocsStats.Builder().count(randomLongBetween(0, 10000))
1368+
.deleted(randomLongBetween(0, 100))
1369+
.totalSizeInBytes(randomLongBetween(0, 1000))
1370+
.build();
1371+
commonStats.store = new StoreStats.Builder().sizeInBytes(randomLongBetween(0, 100))
1372+
.reservedSize(randomLongBetween(0, 1000))
1373+
.build();
13691374
commonStats.indexing = new IndexingStats();
13701375
commonStats.completion = new CompletionStats();
13711376
commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100));

‎server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,13 @@ private ClusterStatsNodeResponse createClusterStatsNodeResponse(
362362

363363
private CommonStats createRandomCommonStats() {
364364
CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE);
365-
commonStats.docs = new DocsStats(randomLongBetween(0, 10000), randomLongBetween(0, 100), randomLongBetween(0, 1000));
366-
commonStats.store = new StoreStats(randomLongBetween(0, 100), randomLongBetween(0, 1000));
365+
commonStats.docs = new DocsStats.Builder().count(randomLongBetween(0, 10000))
366+
.deleted(randomLongBetween(0, 100))
367+
.totalSizeInBytes(randomLongBetween(0, 1000))
368+
.build();
369+
commonStats.store = new StoreStats.Builder().sizeInBytes(randomLongBetween(0, 100))
370+
.reservedSize(randomLongBetween(0, 1000))
371+
.build();
367372
commonStats.indexing = new IndexingStats();
368373
commonStats.completion = new CompletionStats();
369374
commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100));

‎server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,13 @@ private ClusterStatsNodeResponse createClusterStatsNodeResponse(DiscoveryNode no
230230

231231
private CommonStats createRandomCommonStats() {
232232
CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE);
233-
commonStats.docs = new DocsStats(randomLongBetween(0, 10000), randomLongBetween(0, 100), randomLongBetween(0, 1000));
234-
commonStats.store = new StoreStats(randomLongBetween(0, 100), randomLongBetween(0, 1000));
233+
commonStats.docs = new DocsStats.Builder().count(randomLongBetween(0, 10000))
234+
.deleted(randomLongBetween(0, 100))
235+
.totalSizeInBytes(randomLongBetween(0, 1000))
236+
.build();
237+
commonStats.store = new StoreStats.Builder().sizeInBytes(randomLongBetween(0, 100))
238+
.reservedSize(randomLongBetween(0, 1000))
239+
.build();
235240
commonStats.indexing = new IndexingStats();
236241
commonStats.completion = new CompletionStats();
237242
commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100));

‎server/src/test/java/org/opensearch/action/admin/indices/rollover/TransportRolloverActionTests.java‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,19 @@ public void testEvaluateConditions() {
148148
final Set<Condition<?>> conditions = Sets.newHashSet(maxDocsCondition, maxAgeCondition, maxSizeCondition);
149149
Map<String, Boolean> results = evaluateConditions(
150150
conditions,
151-
new DocsStats(matchMaxDocs, 0L, ByteSizeUnit.MB.toBytes(120)),
151+
new DocsStats.Builder().count(matchMaxDocs).deleted(0L).totalSizeInBytes(ByteSizeUnit.MB.toBytes(120)).build(),
152152
metadata
153153
);
154154
assertThat(results.size(), equalTo(3));
155155
for (Boolean matched : results.values()) {
156156
assertThat(matched, equalTo(true));
157157
}
158158

159-
results = evaluateConditions(conditions, new DocsStats(notMatchMaxDocs, 0, notMatchMaxSize.getBytes()), metadata);
159+
results = evaluateConditions(
160+
conditions,
161+
new DocsStats.Builder().count(notMatchMaxDocs).deleted(0).totalSizeInBytes(notMatchMaxSize.getBytes()).build(),
162+
metadata
163+
);
160164
assertThat(results.size(), equalTo(3));
161165
for (Map.Entry<String, Boolean> entry : results.entrySet()) {
162166
if (entry.getKey().equals(maxAgeCondition.toString())) {
@@ -211,7 +215,12 @@ public void testEvaluateWithoutMetadata() {
211215

212216
long matchMaxDocs = randomIntBetween(100, 1000);
213217
final Set<Condition<?>> conditions = Sets.newHashSet(maxDocsCondition, maxAgeCondition, maxSizeCondition);
214-
Map<String, Boolean> results = evaluateConditions(conditions, new DocsStats(matchMaxDocs, 0L, ByteSizeUnit.MB.toBytes(120)), null);
218+
219+
Map<String, Boolean> results = evaluateConditions(
220+
conditions,
221+
new DocsStats.Builder().count(matchMaxDocs).deleted(0L).totalSizeInBytes(ByteSizeUnit.MB.toBytes(120)).build(),
222+
null
223+
);
215224
assertThat(results.size(), equalTo(3));
216225
results.forEach((k, v) -> assertFalse(v));
217226

@@ -390,10 +399,14 @@ public void testResolveIndices() {
390399

391400
private IndicesStatsResponse createIndicesStatResponse(String indexName, long totalDocs, long primariesDocs) {
392401
final CommonStats primaryStats = mock(CommonStats.class);
393-
when(primaryStats.getDocs()).thenReturn(new DocsStats(primariesDocs, 0, between(1, 10000)));
402+
when(primaryStats.getDocs()).thenReturn(
403+
new DocsStats.Builder().count(primariesDocs).deleted(0).totalSizeInBytes(between(1, 10000)).build()
404+
);
394405

395406
final CommonStats totalStats = mock(CommonStats.class);
396-
when(totalStats.getDocs()).thenReturn(new DocsStats(totalDocs, 0, between(1, 10000)));
407+
when(totalStats.getDocs()).thenReturn(
408+
new DocsStats.Builder().count(totalDocs).deleted(0).totalSizeInBytes(between(1, 10000)).build()
409+
);
397410

398411
final IndicesStatsResponse response = mock(IndicesStatsResponse.class);
399412
when(response.getPrimaries()).thenReturn(primaryStats);
@@ -422,10 +435,14 @@ private IndicesStatsResponse createAliasToMultipleIndicesStatsResponse(Map<Strin
422435

423436
private IndexStats createIndexStats(long primaries, long total) {
424437
final CommonStats primariesCommonStats = mock(CommonStats.class);
425-
when(primariesCommonStats.getDocs()).thenReturn(new DocsStats(primaries, 0, between(1, 10000)));
438+
when(primariesCommonStats.getDocs()).thenReturn(
439+
new DocsStats.Builder().count(primaries).deleted(0).totalSizeInBytes(between(1, 10000)).build()
440+
);
426441

427442
final CommonStats totalCommonStats = mock(CommonStats.class);
428-
when(totalCommonStats.getDocs()).thenReturn(new DocsStats(total, 0, between(1, 10000)));
443+
when(totalCommonStats.getDocs()).thenReturn(
444+
new DocsStats.Builder().count(total).deleted(0).totalSizeInBytes(between(1, 10000)).build()
445+
);
429446

430447
IndexStats indexStats = mock(IndexStats.class);
431448
when(indexStats.getPrimaries()).thenReturn(primariesCommonStats);

0 commit comments

Comments
 (0)