diff --git a/CHANGELOG.md b/CHANGELOG.md index 90279bf2b3ae1..51817010ccfcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Pass registry of headers from ActionPlugin.getRestHeaders to ActionPlugin.getRestHandlerWrapper ([#19875](https://github.com/opensearch-project/OpenSearch/pull/19875)) - Refactor the Condition.Stats and DirectoryFileTransferTracker.Stats class to use the Builder pattern instead of constructors ([#19862](https://github.com/opensearch-project/OpenSearch/pull/19862)) - Refactor the RemoteTranslogTransferTracker.Stats and RemoteSegmentTransferTracker.Stats class to use the Builder pattern instead of constructors ([#19837](https://github.com/opensearch-project/OpenSearch/pull/19837)) +- Refactor the GetStats, FlushStats and QueryCacheStats class to use the Builder pattern instead of constructors ([#19935](https://github.com/opensearch-project/OpenSearch/pull/19935)) - Add RangeSemver for `dependencies` in `plugin-descriptor.properties` ([#19939](https://github.com/opensearch-project/OpenSearch/pull/19939)) ### Fixed @@ -89,15 +90,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `com.google.api.grpc:proto-google-iam-v1` from 1.55.0 to 1.57.0 ([#19872](https://github.com/opensearch-project/OpenSearch/pull/19872)) - Bump `org.apache.commons:commons-text` from 1.13.1 to 1.14.0 ([#19871](https://github.com/opensearch-project/OpenSearch/pull/19871)) - Exclude group com.microsoft.sqlserver from hadoop-minicluster ([#19889](https://github.com/opensearch-project/OpenSearch/pull/19889)) -- ### Deprecated +- Bump `actions/github-script` from 7 to 8 ([#19946](https://github.com/opensearch-project/OpenSearch/pull/19946)) +- Bump `com.google.api:gax-httpjson` from 2.69.0 to 2.72.1 ([#19943](https://github.com/opensearch-project/OpenSearch/pull/19943)) + +### Deprecated - Deprecated existing constructors in ThreadPoolStats.Stats in favor of the new Builder ([#19317](https://github.com/opensearch-project/OpenSearch/pull/19317)) - Deprecated existing constructors in IndexingStats.Stats in favor of the new Builder ([#19306](https://github.com/opensearch-project/OpenSearch/pull/19306)) - Deprecated existing constructors in RefreshStats in favor of the new Builder ([#19835](https://github.com/opensearch-project/OpenSearch/pull/19835)) - Deprecated existing constructors in DocStats and StoreStats in favor of the new Builder ([#19863](https://github.com/opensearch-project/OpenSearch/pull/19863)) - Deprecated existing constructors in Condition.Stats and DirectoryFileTransferTracker.Stats in favor of the new Builder ([#19862](https://github.com/opensearch-project/OpenSearch/pull/19862)) - Deprecated existing constructors in RemoteTranslogTransferTracker.Stats and RemoteSegmentTransferTracker.Stats in favor of the new Builder ([#19837](https://github.com/opensearch-project/OpenSearch/pull/19837)) -- Bump `actions/github-script` from 7 to 8 ([#19946](https://github.com/opensearch-project/OpenSearch/pull/19946)) -- Bump `com.google.api:gax-httpjson` from 2.69.0 to 2.72.1 ([#19943](https://github.com/opensearch-project/OpenSearch/pull/19943)) +- Deprecated existing constructors in GetStats, FlushStats and QueryCacheStats in favor of the new Builder ([#19935](https://github.com/opensearch-project/OpenSearch/pull/19935)) ### Removed diff --git a/server/src/main/java/org/opensearch/index/cache/query/QueryCacheStats.java b/server/src/main/java/org/opensearch/index/cache/query/QueryCacheStats.java index 8ca3157d9f775..8548b8a4cb431 100644 --- a/server/src/main/java/org/opensearch/index/cache/query/QueryCacheStats.java +++ b/server/src/main/java/org/opensearch/index/cache/query/QueryCacheStats.java @@ -60,6 +60,19 @@ public class QueryCacheStats implements Writeable, ToXContentFragment { public QueryCacheStats() {} + /** + * Private constructor that takes a builder. + * This is the sole entry point for creating a new QueryCacheStats object. + * @param builder The builder instance containing all the values. + */ + private QueryCacheStats(Builder builder) { + this.ramBytesUsed = builder.ramBytesUsed; + this.hitCount = builder.hitCount; + this.missCount = builder.missCount; + this.cacheCount = builder.cacheCount; + this.cacheSize = builder.cacheSize; + } + public QueryCacheStats(StreamInput in) throws IOException { ramBytesUsed = in.readLong(); hitCount = in.readLong(); @@ -68,6 +81,11 @@ public QueryCacheStats(StreamInput in) throws IOException { cacheSize = in.readLong(); } + /** + * This constructor will be deprecated starting in version 3.4.0. + * Use {@link Builder} instead. + */ + @Deprecated public QueryCacheStats(long ramBytesUsed, long hitCount, long missCount, long cacheCount, long cacheSize) { this.ramBytesUsed = ramBytesUsed; this.hitCount = hitCount; @@ -137,6 +155,53 @@ public long getEvictions() { return cacheCount - cacheSize; } + /** + * Builder for the {@link QueryCacheStats} class. + * Provides a fluent API for constructing a QueryCacheStats object. + */ + public static class Builder { + private long ramBytesUsed = 0; + private long hitCount = 0; + private long missCount = 0; + private long cacheCount = 0; + private long cacheSize = 0; + + public Builder() {} + + public Builder ramBytesUsed(long used) { + this.ramBytesUsed = used; + return this; + } + + public Builder hitCount(long count) { + this.hitCount = count; + return this; + } + + public Builder missCount(long count) { + this.missCount = count; + return this; + } + + public Builder cacheCount(long count) { + this.cacheCount = count; + return this; + } + + public Builder cacheSize(long size) { + this.cacheSize = size; + return this; + } + + /** + * Creates a {@link QueryCacheStats} object from the builder's current state. + * @return A new QueryCacheStats instance. + */ + public QueryCacheStats build() { + return new QueryCacheStats(this); + } + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeLong(ramBytesUsed); diff --git a/server/src/main/java/org/opensearch/index/flush/FlushStats.java b/server/src/main/java/org/opensearch/index/flush/FlushStats.java index 9bce46d1dd9d5..2ae842f9ec0a5 100644 --- a/server/src/main/java/org/opensearch/index/flush/FlushStats.java +++ b/server/src/main/java/org/opensearch/index/flush/FlushStats.java @@ -58,12 +58,28 @@ public FlushStats() { } + /** + * Private constructor that takes a builder. + * This is the sole entry point for creating a new FlushStats object. + * @param builder The builder instance containing all the values. + */ + private FlushStats(Builder builder) { + this.total = builder.total; + this.periodic = builder.periodic; + this.totalTimeInMillis = builder.totalTimeInMillis; + } + public FlushStats(StreamInput in) throws IOException { total = in.readVLong(); totalTimeInMillis = in.readVLong(); periodic = in.readVLong(); } + /** + * This constructor will be deprecated starting in version 3.4.0. + * Use {@link Builder} instead. + */ + @Deprecated public FlushStats(long total, long periodic, long totalTimeInMillis) { this.total = total; this.periodic = periodic; @@ -117,6 +133,42 @@ public TimeValue getTotalTime() { return new TimeValue(totalTimeInMillis); } + /** + * Builder for the {@link FlushStats} class. + * Provides a fluent API for constructing a FlushStats object. + */ + public static class Builder { + private long total = 0; + private long periodic = 0; + private long totalTimeInMillis = 0; + + public Builder() {} + + public Builder total(long total) { + this.total = total; + return this; + } + + public Builder periodic(long periodic) { + this.periodic = periodic; + return this; + } + + public Builder totalTimeInMillis(long time) { + this.totalTimeInMillis = time; + return this; + } + + /** + * Creates a {@link FlushStats} object from the builder's current state. + * + * @return A new FlushStats instance. + */ + public FlushStats build() { + return new FlushStats(this); + } + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.FLUSH); diff --git a/server/src/main/java/org/opensearch/index/get/GetStats.java b/server/src/main/java/org/opensearch/index/get/GetStats.java index 55f14294d774b..dae272f460fb7 100644 --- a/server/src/main/java/org/opensearch/index/get/GetStats.java +++ b/server/src/main/java/org/opensearch/index/get/GetStats.java @@ -59,6 +59,19 @@ public class GetStats implements Writeable, ToXContentFragment { public GetStats() {} + /** + * Private constructor that takes a builder. + * This is the sole entry point for creating a new GetStats object. + * @param builder The builder instance containing all the values. + */ + private GetStats(Builder builder) { + this.existsCount = builder.existsCount; + this.existsTimeInMillis = builder.existsTimeInMillis; + this.missingCount = builder.missingCount; + this.missingTimeInMillis = builder.missingTimeInMillis; + this.current = builder.current; + } + public GetStats(StreamInput in) throws IOException { existsCount = in.readVLong(); existsTimeInMillis = in.readVLong(); @@ -67,6 +80,11 @@ public GetStats(StreamInput in) throws IOException { current = in.readVLong(); } + /** + * This constructor will be deprecated starting in version 3.4.0. + * Use {@link Builder} instead. + */ + @Deprecated public GetStats(long existsCount, long existsTimeInMillis, long missingCount, long missingTimeInMillis, long current) { this.existsCount = existsCount; this.existsTimeInMillis = existsTimeInMillis; @@ -134,6 +152,54 @@ public long current() { return this.current; } + /** + * Builder for the {@link GetStats} class. + * Provides a fluent API for constructing a GetStats object. + */ + public static class Builder { + private long existsCount = 0; + private long existsTimeInMillis = 0; + private long missingCount = 0; + private long missingTimeInMillis = 0; + private long current = 0; + + public Builder() {} + + public Builder existsCount(long count) { + this.existsCount = count; + return this; + } + + public Builder existsTimeInMillis(long time) { + this.existsTimeInMillis = time; + return this; + } + + public Builder missingCount(long count) { + this.missingCount = count; + return this; + } + + public Builder missingTimeInMillis(long time) { + this.missingTimeInMillis = time; + return this; + } + + public Builder current(long current) { + this.current = current; + return this; + } + + /** + * Creates a {@link GetStats} object from the builder's current state. + * + * @return A new GetStats instance. + */ + public GetStats build() { + return new GetStats(this); + } + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.GET); diff --git a/server/src/main/java/org/opensearch/index/get/ShardGetService.java b/server/src/main/java/org/opensearch/index/get/ShardGetService.java index 082a06c8f7773..c4d765c4e0358 100644 --- a/server/src/main/java/org/opensearch/index/get/ShardGetService.java +++ b/server/src/main/java/org/opensearch/index/get/ShardGetService.java @@ -107,13 +107,12 @@ public ShardGetService(IndexSettings indexSettings, IndexShard indexShard, Mappe } public GetStats stats() { - return new GetStats( - existsMetric.count(), - TimeUnit.NANOSECONDS.toMillis(existsMetric.sum()), - missingMetric.count(), - TimeUnit.NANOSECONDS.toMillis(missingMetric.sum()), - currentMetric.count() - ); + return new GetStats.Builder().existsCount(existsMetric.count()) + .existsTimeInMillis(TimeUnit.NANOSECONDS.toMillis(existsMetric.sum())) + .missingCount(missingMetric.count()) + .missingTimeInMillis(TimeUnit.NANOSECONDS.toMillis(missingMetric.sum())) + .current(currentMetric.count()) + .build(); } public GetResult get( diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 72be6d79aabee..f3fe60d70d532 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -1513,7 +1513,10 @@ public RefreshStats refreshStats() { } public FlushStats flushStats() { - return new FlushStats(flushMetric.count(), periodicFlushMetric.count(), TimeUnit.NANOSECONDS.toMillis(flushMetric.sum())); + return new FlushStats.Builder().total(flushMetric.count()) + .periodic(periodicFlushMetric.count()) + .totalTimeInMillis(TimeUnit.NANOSECONDS.toMillis(flushMetric.sum())) + .build(); } public DocsStats docStats() { diff --git a/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java b/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java index 37fb276c3b240..eeaafb3225580 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java @@ -183,7 +183,9 @@ public QueryCacheStats getStats(ShardId shard) { // We also have some shared ram usage that we try to distribute to // proportionally to their number of cache entries of each shard if (stats.isEmpty()) { - shardStats.add(new QueryCacheStats(sharedRamBytesUsed, 0, 0, 0, 0)); + shardStats.add( + new QueryCacheStats.Builder().ramBytesUsed(sharedRamBytesUsed).hitCount(0).missCount(0).cacheCount(0).cacheSize(0).build() + ); } else { long totalSize = 0; for (QueryCacheStats s : stats.values()) { @@ -191,7 +193,14 @@ public QueryCacheStats getStats(ShardId shard) { } final double weight = totalSize == 0 ? 1d / stats.size() : ((double) shardStats.getCacheSize()) / totalSize; final long additionalRamBytesUsed = Math.round(weight * sharedRamBytesUsed); - shardStats.add(new QueryCacheStats(additionalRamBytesUsed, 0, 0, 0, 0)); + shardStats.add( + new QueryCacheStats.Builder().ramBytesUsed(additionalRamBytesUsed) + .hitCount(0) + .missCount(0) + .cacheCount(0) + .cacheSize(0) + .build() + ); } return shardStats; } @@ -287,7 +296,12 @@ private static class Stats implements Cloneable { } QueryCacheStats toQueryCacheStats() { - return new QueryCacheStats(ramBytesUsed, hitCount, missCount, cacheCount, cacheSize); + return new QueryCacheStats.Builder().ramBytesUsed(ramBytesUsed) + .hitCount(hitCount) + .missCount(missCount) + .cacheCount(cacheCount) + .cacheSize(cacheSize) + .build(); } @Override diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index 957119ed202e9..34a041c3620db 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1372,15 +1372,17 @@ private CommonStats createRandomCommonStats() { .build(); commonStats.indexing = new IndexingStats(); commonStats.completion = new CompletionStats(); - commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100)); + commonStats.flush = new FlushStats.Builder().total(randomLongBetween(0, 100)) + .periodic(randomLongBetween(0, 100)) + .totalTimeInMillis(randomLongBetween(0, 100)) + .build(); commonStats.fieldData = new FieldDataStats(randomLongBetween(0, 100), randomLongBetween(0, 100), null); - commonStats.queryCache = new QueryCacheStats( - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100) - ); + commonStats.queryCache = new QueryCacheStats.Builder().ramBytesUsed(randomLongBetween(0, 100)) + .hitCount(randomLongBetween(0, 100)) + .missCount(randomLongBetween(0, 100)) + .cacheCount(randomLongBetween(0, 100)) + .cacheSize(randomLongBetween(0, 100)) + .build(); commonStats.segments = new SegmentsStats(); return commonStats; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java index 4498d4c73912c..0bbcad0407c4d 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java @@ -371,15 +371,17 @@ private CommonStats createRandomCommonStats() { .build(); commonStats.indexing = new IndexingStats(); commonStats.completion = new CompletionStats(); - commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100)); + commonStats.flush = new FlushStats.Builder().total(randomLongBetween(0, 100)) + .periodic(randomLongBetween(0, 100)) + .totalTimeInMillis(randomLongBetween(0, 100)) + .build(); commonStats.fieldData = new FieldDataStats(randomLongBetween(0, 100), randomLongBetween(0, 100), null); - commonStats.queryCache = new QueryCacheStats( - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100) - ); + commonStats.queryCache = new QueryCacheStats.Builder().ramBytesUsed(randomLongBetween(0, 100)) + .hitCount(randomLongBetween(0, 100)) + .missCount(randomLongBetween(0, 100)) + .cacheCount(randomLongBetween(0, 100)) + .cacheSize(randomLongBetween(0, 100)) + .build(); commonStats.segments = new SegmentsStats(); return commonStats; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java index c8ddd5c6b4eef..f673dcf56f7cd 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsResponseTests.java @@ -239,15 +239,17 @@ private CommonStats createRandomCommonStats() { .build(); commonStats.indexing = new IndexingStats(); commonStats.completion = new CompletionStats(); - commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100)); + commonStats.flush = new FlushStats.Builder().total(randomLongBetween(0, 100)) + .periodic(randomLongBetween(0, 100)) + .totalTimeInMillis(randomLongBetween(0, 100)) + .build(); commonStats.fieldData = new FieldDataStats(randomLongBetween(0, 100), randomLongBetween(0, 100), null); - commonStats.queryCache = new QueryCacheStats( - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100), - randomLongBetween(0, 100) - ); + commonStats.queryCache = new QueryCacheStats.Builder().ramBytesUsed(randomLongBetween(0, 100)) + .hitCount(randomLongBetween(0, 100)) + .missCount(randomLongBetween(0, 100)) + .cacheCount(randomLongBetween(0, 100)) + .cacheSize(randomLongBetween(0, 100)) + .build(); commonStats.segments = new SegmentsStats(); return commonStats;