diff --git a/CHANGELOG.md b/CHANGELOG.md index 254c35c66998b..143c6b84e50cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Refactor the RefreshStats class to use the Builder pattern instead of constructors ([#19835](https://github.com/opensearch-project/OpenSearch/pull/19835)) - Refactor the DocStats and StoreStats class to use the Builder pattern instead of constructors ([#19863](https://github.com/opensearch-project/OpenSearch/pull/19863)) - 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)) ### Fixed @@ -92,6 +93,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - 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)) ### Removed diff --git a/server/src/main/java/org/opensearch/action/admin/indices/rollover/Condition.java b/server/src/main/java/org/opensearch/action/admin/indices/rollover/Condition.java index 4d0b7fc8c13c7..6870e514d6ed3 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/rollover/Condition.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/rollover/Condition.java @@ -106,11 +106,62 @@ public static class Stats { public final long indexCreated; public final ByteSizeValue indexSize; + /** + * 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.numDocs = builder.numDocs; + this.indexCreated = builder.indexCreated; + this.indexSize = builder.indexSize; + } + + /** + * This constructor will be deprecated starting in version 3.4.0. + * Use {@link Builder} instead. + */ + @Deprecated public Stats(long numDocs, long indexCreated, ByteSizeValue indexSize) { this.numDocs = numDocs; this.indexCreated = indexCreated; this.indexSize = indexSize; } + + /** + * Builder for the {@link Stats} class. + * Provides a fluent API for constructing a Stats object. + */ + public static class Builder { + private long numDocs = 0; + private long indexCreated = 0; + private ByteSizeValue indexSize = null; + + public Builder() {} + + public Builder numDocs(long numDocs) { + this.numDocs = numDocs; + return this; + } + + public Builder indexCreated(long indexCreated) { + this.indexCreated = indexCreated; + return this; + } + + public Builder indexSize(ByteSizeValue size) { + this.indexSize = size; + 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); + } + } } /** diff --git a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java index 876cf84c88e5a..dc768df1f0c8b 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -304,7 +304,10 @@ static Map evaluateConditions( } final long numDocs = docsStats == null ? 0 : docsStats.getCount(); final long indexSize = docsStats == null ? 0 : docsStats.getTotalSizeInBytes(); - final Condition.Stats stats = new Condition.Stats(numDocs, metadata.getCreationDate(), new ByteSizeValue(indexSize)); + final Condition.Stats stats = new Condition.Stats.Builder().numDocs(numDocs) + .indexCreated(metadata.getCreationDate()) + .indexSize(new ByteSizeValue(indexSize)) + .build(); return conditions.stream() .map(condition -> condition.evaluate(stats)) .collect(Collectors.toMap(result -> result.condition.toString(), result -> result.matched)); diff --git a/server/src/main/java/org/opensearch/index/store/DirectoryFileTransferTracker.java b/server/src/main/java/org/opensearch/index/store/DirectoryFileTransferTracker.java index 2ab615677dedb..6036f8a5b709c 100644 --- a/server/src/main/java/org/opensearch/index/store/DirectoryFileTransferTracker.java +++ b/server/src/main/java/org/opensearch/index/store/DirectoryFileTransferTracker.java @@ -153,16 +153,15 @@ public DirectoryFileTransferTracker() { } public DirectoryFileTransferTracker.Stats stats() { - return new Stats( - transferredBytesStarted.get(), - transferredBytesFailed.get(), - transferredBytesSucceeded.get(), - lastTransferTimestampMs.get(), - totalTransferTimeInMs.get(), - transferredBytesMovingAverageReference.get().getAverage(), - lastSuccessfulTransferInBytes.get(), - transferredBytesPerSecMovingAverageReference.get().getAverage() - ); + return new Stats.Builder().transferredBytesStarted(transferredBytesStarted.get()) + .transferredBytesFailed(transferredBytesFailed.get()) + .transferredBytesSucceeded(transferredBytesSucceeded.get()) + .lastTransferTimestampMs(lastTransferTimestampMs.get()) + .totalTransferTimeInMs(totalTransferTimeInMs.get()) + .transferredBytesMovingAverage(transferredBytesMovingAverageReference.get().getAverage()) + .lastSuccessfulTransferInBytes(lastSuccessfulTransferInBytes.get()) + .transferredBytesPerSecMovingAverage(transferredBytesPerSecMovingAverageReference.get().getAverage()) + .build(); } /** @@ -181,6 +180,27 @@ public static class Stats implements Writeable { public final long lastSuccessfulTransferInBytes; public final double transferredBytesPerSecMovingAverage; + /** + * 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.transferredBytesStarted = builder.transferredBytesStarted; + this.transferredBytesFailed = builder.transferredBytesFailed; + this.transferredBytesSucceeded = builder.transferredBytesSucceeded; + this.lastTransferTimestampMs = builder.lastTransferTimestampMs; + this.totalTransferTimeInMs = builder.totalTransferTimeInMs; + this.transferredBytesMovingAverage = builder.transferredBytesMovingAverage; + this.lastSuccessfulTransferInBytes = builder.lastSuccessfulTransferInBytes; + this.transferredBytesPerSecMovingAverage = builder.transferredBytesPerSecMovingAverage; + } + + /** + * This constructor will be deprecated starting in version 3.4.0. + * Use {@link Builder} instead. + */ + @Deprecated public Stats( long transferredBytesStarted, long transferredBytesFailed, @@ -212,6 +232,71 @@ public Stats(StreamInput in) throws IOException { this.transferredBytesPerSecMovingAverage = in.readDouble(); } + /** + * Builder for the {@link Stats} class. + * Provides a fluent API for constructing a Stats object. + */ + public static class Builder { + private long transferredBytesStarted = 0; + private long transferredBytesFailed = 0; + private long transferredBytesSucceeded = 0; + private long lastTransferTimestampMs = 0; + private long totalTransferTimeInMs = 0; + private double transferredBytesMovingAverage = 0; + private long lastSuccessfulTransferInBytes = 0; + private double transferredBytesPerSecMovingAverage = 0; + + public Builder() {} + + public Builder transferredBytesStarted(long started) { + this.transferredBytesStarted = started; + return this; + } + + public Builder transferredBytesFailed(long failed) { + this.transferredBytesFailed = failed; + return this; + } + + public Builder transferredBytesSucceeded(long succeeded) { + this.transferredBytesSucceeded = succeeded; + return this; + } + + public Builder lastTransferTimestampMs(long timestamp) { + this.lastTransferTimestampMs = timestamp; + return this; + } + + public Builder totalTransferTimeInMs(long time) { + this.totalTransferTimeInMs = time; + return this; + } + + public Builder transferredBytesMovingAverage(double average) { + this.transferredBytesMovingAverage = average; + return this; + } + + public Builder lastSuccessfulTransferInBytes(long bytes) { + this.lastSuccessfulTransferInBytes = bytes; + return this; + } + + public Builder transferredBytesPerSecMovingAverage(double average) { + this.transferredBytesPerSecMovingAverage = average; + 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); + } + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeLong(transferredBytesStarted); diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/RemoteStoreStatsTestHelper.java b/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/RemoteStoreStatsTestHelper.java index 93eb011424e5c..38dfb5e58993f 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/RemoteStoreStatsTestHelper.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/remotestore/stats/RemoteStoreStatsTestHelper.java @@ -103,11 +103,27 @@ static RemoteSegmentTransferTracker.Stats createStatsForRemoteStoreRestoredPrima } static DirectoryFileTransferTracker.Stats createSampleDirectoryFileTransferStats() { - return new DirectoryFileTransferTracker.Stats(10, 0, 10, 12345, 5, 5, 5, 10); + return new DirectoryFileTransferTracker.Stats.Builder().transferredBytesStarted(10) + .transferredBytesFailed(0) + .transferredBytesSucceeded(10) + .lastTransferTimestampMs(12345) + .totalTransferTimeInMs(5) + .transferredBytesMovingAverage(5) + .lastSuccessfulTransferInBytes(5) + .transferredBytesPerSecMovingAverage(10) + .build(); } static DirectoryFileTransferTracker.Stats createZeroDirectoryFileTransferStats() { - return new DirectoryFileTransferTracker.Stats(0, 0, 0, 0, 0, 0, 0, 0); + return new DirectoryFileTransferTracker.Stats.Builder().transferredBytesStarted(0) + .transferredBytesFailed(0) + .transferredBytesSucceeded(0) + .lastTransferTimestampMs(0) + .totalTransferTimeInMs(0) + .transferredBytesMovingAverage(0) + .lastSuccessfulTransferInBytes(0) + .transferredBytesPerSecMovingAverage(0) + .build(); } static ShardRouting createShardRouting(ShardId shardId, boolean isPrimary) { diff --git a/server/src/test/java/org/opensearch/action/admin/indices/rollover/ConditionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/rollover/ConditionTests.java index 3d32d2e8365e7..fe1339740fb97 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/rollover/ConditionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/rollover/ConditionTests.java @@ -46,12 +46,16 @@ public void testMaxAge() { final MaxAgeCondition maxAgeCondition = new MaxAgeCondition(TimeValue.timeValueHours(1)); long indexCreatedMatch = System.currentTimeMillis() - TimeValue.timeValueMinutes(61).getMillis(); - Condition.Result evaluate = maxAgeCondition.evaluate(new Condition.Stats(0, indexCreatedMatch, randomByteSize())); + Condition.Result evaluate = maxAgeCondition.evaluate( + new Condition.Stats.Builder().numDocs(0).indexCreated(indexCreatedMatch).indexSize(randomByteSize()).build() + ); assertThat(evaluate.condition, equalTo(maxAgeCondition)); assertThat(evaluate.matched, equalTo(true)); long indexCreatedNotMatch = System.currentTimeMillis() - TimeValue.timeValueMinutes(59).getMillis(); - evaluate = maxAgeCondition.evaluate(new Condition.Stats(0, indexCreatedNotMatch, randomByteSize())); + evaluate = maxAgeCondition.evaluate( + new Condition.Stats.Builder().numDocs(0).indexCreated(indexCreatedNotMatch).indexSize(randomByteSize()).build() + ); assertThat(evaluate.condition, equalTo(maxAgeCondition)); assertThat(evaluate.matched, equalTo(false)); } @@ -60,12 +64,16 @@ public void testMaxDocs() { final MaxDocsCondition maxDocsCondition = new MaxDocsCondition(100L); long maxDocsMatch = randomIntBetween(100, 1000); - Condition.Result evaluate = maxDocsCondition.evaluate(new Condition.Stats(maxDocsMatch, 0, randomByteSize())); + Condition.Result evaluate = maxDocsCondition.evaluate( + new Condition.Stats.Builder().numDocs(maxDocsMatch).indexCreated(0).indexSize(randomByteSize()).build() + ); assertThat(evaluate.condition, equalTo(maxDocsCondition)); assertThat(evaluate.matched, equalTo(true)); long maxDocsNotMatch = randomIntBetween(0, 99); - evaluate = maxDocsCondition.evaluate(new Condition.Stats(0, maxDocsNotMatch, randomByteSize())); + evaluate = maxDocsCondition.evaluate( + new Condition.Stats.Builder().numDocs(0).indexCreated(maxDocsNotMatch).indexSize(randomByteSize()).build() + ); assertThat(evaluate.condition, equalTo(maxDocsCondition)); assertThat(evaluate.matched, equalTo(false)); } @@ -74,25 +82,26 @@ public void testMaxSize() { MaxSizeCondition maxSizeCondition = new MaxSizeCondition(new ByteSizeValue(randomIntBetween(10, 20), ByteSizeUnit.MB)); Condition.Result result = maxSizeCondition.evaluate( - new Condition.Stats(randomNonNegativeLong(), randomNonNegativeLong(), new ByteSizeValue(0, ByteSizeUnit.MB)) + new Condition.Stats.Builder().numDocs(randomNonNegativeLong()) + .indexCreated(randomNonNegativeLong()) + .indexSize(new ByteSizeValue(0, ByteSizeUnit.MB)) + .build() ); assertThat(result.matched, equalTo(false)); result = maxSizeCondition.evaluate( - new Condition.Stats( - randomNonNegativeLong(), - randomNonNegativeLong(), - new ByteSizeValue(randomIntBetween(0, 9), ByteSizeUnit.MB) - ) + new Condition.Stats.Builder().numDocs(randomNonNegativeLong()) + .indexCreated(randomNonNegativeLong()) + .indexSize(new ByteSizeValue(randomIntBetween(0, 9), ByteSizeUnit.MB)) + .build() ); assertThat(result.matched, equalTo(false)); result = maxSizeCondition.evaluate( - new Condition.Stats( - randomNonNegativeLong(), - randomNonNegativeLong(), - new ByteSizeValue(randomIntBetween(20, 1000), ByteSizeUnit.MB) - ) + new Condition.Stats.Builder().numDocs(randomNonNegativeLong()) + .indexCreated(randomNonNegativeLong()) + .indexSize(new ByteSizeValue(randomIntBetween(20, 1000), ByteSizeUnit.MB)) + .build() ); assertThat(result.matched, equalTo(true)); }