diff --git a/CHANGELOG.md b/CHANGELOG.md index fa393bc30dd28..d2ce4a7f7cae9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Remove MultiCollectorWrapper and use MultiCollector in Lucene instead ([#19595](https://github.com/opensearch-project/OpenSearch/pull/19595)) - Change implementation for `percentiles` aggregation for latency improvement ([#19648](https://github.com/opensearch-project/OpenSearch/pull/19648)) - Refactor the ThreadPoolStats.Stats class to use the Builder pattern instead of constructors ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306)) +- Refactor the IndexingStats.Stats class to use the Builder pattern instead of constructors ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306)) ### Fixed - Fix Allocation and Rebalance Constraints of WeightFunction are incorrectly reset ([#19012](https://github.com/opensearch-project/OpenSearch/pull/19012)) @@ -46,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Deprecated - Deprecated existing constructors in ThreadPoolStats.Stats in favor of the new Builder ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306)) +- Deprecated existing constructors in IndexingStats.Stats in favor of the new Builder ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306)) ### Removed diff --git a/server/src/main/java/org/opensearch/index/shard/IndexingStats.java b/server/src/main/java/org/opensearch/index/shard/IndexingStats.java index 60a93814da0e2..ebad62b69199f 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexingStats.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexingStats.java @@ -165,6 +165,26 @@ public void writeTo(StreamOutput out) throws IOException { docStatusStats = new DocStatusStats(); } + /** + * Private constructor that takes a builder. + * This is the sole entry point for creating a new Stats object. + * @param builder The builder instance containing all the values. + */ + private Stats(Builder builder) { + this.indexCount = builder.indexCount; + this.indexTimeInMillis = builder.indexTimeInMillis; + this.indexCurrent = builder.indexCurrent; + this.indexFailedCount = builder.indexFailedCount; + this.deleteCount = builder.deleteCount; + this.deleteTimeInMillis = builder.deleteTimeInMillis; + this.deleteCurrent = builder.deleteCurrent; + this.noopUpdateCount = builder.noopUpdateCount; + this.throttleTimeInMillis = builder.throttleTimeInMillis; + this.isThrottled = builder.isThrottled; + this.docStatusStats = builder.docStatusStats; + this.maxLastIndexRequestTimestamp = builder.maxLastIndexRequestTimestamp; + } + public Stats(StreamInput in) throws IOException { indexCount = in.readVLong(); indexTimeInMillis = in.readVLong(); @@ -188,6 +208,11 @@ public Stats(StreamInput in) throws IOException { } } + /** + * This constructor will be deprecated starting in version 3.3.0. + * Use {@link Builder} instead. + */ + @Deprecated public Stats( long indexCount, long indexTimeInMillis, @@ -217,6 +242,11 @@ public Stats( ); } + /** + * This constructor will be deprecated starting in version 3.3.0. + * Use {@link Builder} instead. + */ + @Deprecated public Stats( long indexCount, long indexTimeInMillis, @@ -386,6 +416,94 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } + /** + * Builder for the {@link Stats} class. + * Provides a fluent API for constructing a Stats object. + */ + public static class Builder { + private long indexCount = 0; + private long indexTimeInMillis = 0; + private long indexCurrent = 0; + private long indexFailedCount = 0; + private long deleteCount = 0; + private long deleteTimeInMillis = 0; + private long deleteCurrent = 0; + private long noopUpdateCount = 0; + private long throttleTimeInMillis = 0; + private boolean isThrottled = false; + private DocStatusStats docStatusStats = null; + private long maxLastIndexRequestTimestamp = 0; + + public Builder() {} + + public Builder indexCount(long count) { + this.indexCount = count; + return this; + } + + public Builder indexTimeInMillis(long time) { + this.indexTimeInMillis = time; + return this; + } + + public Builder indexCurrent(long current) { + this.indexCurrent = current; + return this; + } + + public Builder indexFailedCount(long count) { + this.indexFailedCount = count; + return this; + } + + public Builder deleteCount(long count) { + this.deleteCount = count; + return this; + } + + public Builder deleteTimeInMillis(long time) { + this.deleteTimeInMillis = time; + return this; + } + + public Builder deleteCurrent(long current) { + this.deleteCurrent = current; + return this; + } + + public Builder noopUpdateCount(long count) { + this.noopUpdateCount = count; + return this; + } + + public Builder throttleTimeInMillis(long time) { + this.throttleTimeInMillis = time; + return this; + } + + public Builder isThrottled(boolean throttled) { + this.isThrottled = throttled; + return this; + } + + public Builder docStatusStats(DocStatusStats stats) { + this.docStatusStats = stats; + return this; + } + + public Builder maxLastIndexRequestTimestamp(long timestamp) { + this.maxLastIndexRequestTimestamp = timestamp; + return this; + } + + /** + * Creates a {@link Stats} object from the builder's current state. + * @return A new Stats instance. + */ + public Stats build() { + return new Stats(this); + } + } } private final Stats totalStats; diff --git a/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java b/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java index 9c901eacb1cc5..7fd67a9206397 100644 --- a/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java +++ b/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java @@ -154,20 +154,19 @@ static class StatsHolder { private final MaxMetric maxLastIndexRequestTimestamp = new MaxMetric(); IndexingStats.Stats stats(boolean isThrottled, long currentThrottleMillis) { - return new IndexingStats.Stats( - indexMetric.count(), - TimeUnit.NANOSECONDS.toMillis(indexMetric.sum()), - indexCurrent.count(), - indexFailed.count(), - deleteMetric.count(), - TimeUnit.NANOSECONDS.toMillis(deleteMetric.sum()), - deleteCurrent.count(), - noopUpdates.count(), - isThrottled, - TimeUnit.MILLISECONDS.toMillis(currentThrottleMillis), - new IndexingStats.Stats.DocStatusStats(), - maxLastIndexRequestTimestamp.get() - ); + return new IndexingStats.Stats.Builder().indexCount(indexMetric.count()) + .indexTimeInMillis(TimeUnit.NANOSECONDS.toMillis(indexMetric.sum())) + .indexCurrent(indexCurrent.count()) + .indexFailedCount(indexFailed.count()) + .deleteCount(deleteMetric.count()) + .deleteTimeInMillis(TimeUnit.NANOSECONDS.toMillis(deleteMetric.sum())) + .deleteCurrent(deleteCurrent.count()) + .noopUpdateCount(noopUpdates.count()) + .isThrottled(isThrottled) + .throttleTimeInMillis(TimeUnit.MILLISECONDS.toMillis(currentThrottleMillis)) + .docStatusStats(new IndexingStats.Stats.DocStatusStats()) + .maxLastIndexRequestTimestamp(maxLastIndexRequestTimestamp.get()) + .build(); } } } diff --git a/server/src/test/java/org/opensearch/index/shard/IndexingStatsTests.java b/server/src/test/java/org/opensearch/index/shard/IndexingStatsTests.java index aafa58e3791a9..26b7f4b7a1ebe 100644 --- a/server/src/test/java/org/opensearch/index/shard/IndexingStatsTests.java +++ b/server/src/test/java/org/opensearch/index/shard/IndexingStatsTests.java @@ -128,9 +128,20 @@ public void testMaxLastIndexRequestTimestampAggregation() throws Exception { long ts1 = randomLongBetween(0, 1000000); long ts2 = randomLongBetween(0, 1000000); long ts3 = randomLongBetween(0, 1000000); - IndexingStats.Stats stats1 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts1); - IndexingStats.Stats stats2 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts2); - IndexingStats.Stats stats3 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts3); + IndexingStats.Stats.Builder defaultStats = new IndexingStats.Stats.Builder().indexCount(1) + .indexTimeInMillis(2) + .indexCurrent(3) + .indexFailedCount(4) + .deleteCount(5) + .deleteTimeInMillis(6) + .deleteCurrent(7) + .noopUpdateCount(8) + .isThrottled(false) + .throttleTimeInMillis(9) + .docStatusStats(docStatusStats); + IndexingStats.Stats stats1 = defaultStats.maxLastIndexRequestTimestamp(ts1).build(); + IndexingStats.Stats stats2 = defaultStats.maxLastIndexRequestTimestamp(ts2).build(); + IndexingStats.Stats stats3 = defaultStats.maxLastIndexRequestTimestamp(ts3).build(); // Aggregate stats1 + stats2 stats1.add(stats2); @@ -141,12 +152,13 @@ public void testMaxLastIndexRequestTimestampAggregation() throws Exception { assertEquals(Math.max(Math.max(ts1, ts2), ts3), stats1.getMaxLastIndexRequestTimestamp()); // Test with zero and negative values - IndexingStats.Stats statsZero = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, 0L); - IndexingStats.Stats statsNeg = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, -100L); + IndexingStats.Stats statsZero = defaultStats.maxLastIndexRequestTimestamp(0L).build(); + IndexingStats.Stats statsNeg = defaultStats.maxLastIndexRequestTimestamp(-100L).build(); + statsZero.add(statsNeg); assertEquals(0L, statsZero.getMaxLastIndexRequestTimestamp()); - IndexingStats.Stats statsNeg2 = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, -50L); + IndexingStats.Stats statsNeg2 = defaultStats.maxLastIndexRequestTimestamp(-50L).build(); statsNeg.add(statsNeg2); assertEquals(-50L, statsNeg.getMaxLastIndexRequestTimestamp()); } @@ -154,7 +166,19 @@ public void testMaxLastIndexRequestTimestampAggregation() throws Exception { public void testMaxLastIndexRequestTimestampBackwardCompatibility() throws IOException { IndexingStats.Stats.DocStatusStats docStatusStats = new IndexingStats.Stats.DocStatusStats(); long ts = randomLongBetween(0, 1000000); - IndexingStats.Stats stats = new IndexingStats.Stats(1, 2, 3, 4, 5, 6, 7, 8, false, 9, docStatusStats, ts); + IndexingStats.Stats stats = new IndexingStats.Stats.Builder().indexCount(1) + .indexTimeInMillis(2) + .indexCurrent(3) + .indexFailedCount(4) + .deleteCount(5) + .deleteTimeInMillis(6) + .deleteCurrent(7) + .noopUpdateCount(8) + .isThrottled(false) + .throttleTimeInMillis(9) + .docStatusStats(docStatusStats) + .maxLastIndexRequestTimestamp(ts) + .build(); // Serialize with V_3_1_0 (should include the field) BytesStreamOutput outNew = new BytesStreamOutput(); @@ -181,20 +205,19 @@ private IndexingStats createTestInstance() { docStatusStats.add(RestStatus.fromCode(i * 100), randomNonNegativeLong()); } - IndexingStats.Stats stats = new IndexingStats.Stats( - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomBoolean(), - randomNonNegativeLong(), - docStatusStats, - randomLong() - ); + IndexingStats.Stats stats = new IndexingStats.Stats.Builder().indexCount(randomNonNegativeLong()) + .indexTimeInMillis(randomNonNegativeLong()) + .indexCurrent(randomNonNegativeLong()) + .indexFailedCount(randomNonNegativeLong()) + .deleteCount(randomNonNegativeLong()) + .deleteTimeInMillis(randomNonNegativeLong()) + .deleteCurrent(randomNonNegativeLong()) + .noopUpdateCount(randomNonNegativeLong()) + .isThrottled(randomBoolean()) + .throttleTimeInMillis(randomNonNegativeLong()) + .docStatusStats(docStatusStats) + .maxLastIndexRequestTimestamp(randomLong()) + .build(); return new IndexingStats(stats); }