From 16052d4fa5cf003d0243518f41c08c51a8002314 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 11 Jun 2024 12:58:50 +0200 Subject: [PATCH] feedback --- .../blobcache/common/ProgressListenableActionFuture.java | 3 ++- .../common/ProgressListenableActionFutureTests.java | 7 +++++-- .../blobcache/common/SparseFileTrackerTests.java | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/common/ProgressListenableActionFuture.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/common/ProgressListenableActionFuture.java index d5c5c61b1ad19..00cc9554a64eb 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/common/ProgressListenableActionFuture.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/common/ProgressListenableActionFuture.java @@ -37,7 +37,8 @@ private record PositionAndListener(long position, ActionListener listener) /** * A consumer that accepts progress made by this {@link ProgressListenableActionFuture}. The consumer is called before listeners are - * notified of the updated progress value in {@link #onProgress(long)}. The consumer can be called with out-of-order progress values. + * notified of the updated progress value in {@link #onProgress(long)} if the value is less than the actual end. The consumer can be + * called with out-of-order progress values. */ @Nullable private final LongConsumer progressConsumer; diff --git a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/ProgressListenableActionFutureTests.java b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/ProgressListenableActionFutureTests.java index 4734d19b07c76..4490d087cec1f 100644 --- a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/ProgressListenableActionFutureTests.java +++ b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/ProgressListenableActionFutureTests.java @@ -243,11 +243,12 @@ public void testLongConsumerCalledOnProgressUpdate() { var future = new ProgressListenableActionFuture( start, end, - p -> assertThat("LongConsumer should not consumed the same value twice", consumed.add(p), equalTo(true)) + p -> assertThat("LongConsumer should not consumed the same value twice", consumed.add(p), equalTo(true)) ); long position = start; - for (int i = 0; i < 25 && position < end - 1L; i++) { + int iters = randomIntBetween(10, 25); + for (int i = 0; i < iters && position < end - 1L; i++) { var progress = randomLongBetween(position + 1L, end - 1L); var listener = new PlainActionFuture(); @@ -268,6 +269,8 @@ public void testLongConsumerCalledOnProgressUpdate() { assertThat(listener.isDone(), equalTo(true)); position = progress; } + future.onProgress(end); + assertThat("LongConsumer is not called when progress is updated to the end", consumed.contains(end), equalTo(false)); } private static ProgressListenableActionFuture randomFuture() { diff --git a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/SparseFileTrackerTests.java b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/SparseFileTrackerTests.java index 0b16b70fd3596..fda560ccb2e21 100644 --- a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/SparseFileTrackerTests.java +++ b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/common/SparseFileTrackerTests.java @@ -562,6 +562,7 @@ public void testCompletePointerUpdatesOnProgress() { tracker.getComplete(), equalTo(j + 1L) ); + assertThat(awaitingListener.isDone(), equalTo(true)); latestUpdatedCompletePointer = tracker.getComplete(); } else { assertThat(