Skip to content

Commit d1cc5c7

Browse files
committed
Don't rely on test code execution time span.
Current implementation of [`RemoteSegmentTransferTrackerTests.testComputeTimeLagOnUpdate()`](https://github.com/opensearch-project/OpenSearch/blob/2b17902643738f0d2a75ade7c85cbca94d18ce49/server/src/test/java/org/opensearch/index/remote/RemoteSegmentTransferTrackerTests.java#L139) test rely on some assumptions about how fast the testing code will finish in JVM. Moreover it does not precisely control boundaries of the time span, specifically the start of the span because it is determined by internal implementation of [`RemoteSegmentTransferTracker.getTimeMsLag()`](https://github.com/opensearch-project/OpenSearch/blob/2b17902643738f0d2a75ade7c85cbca94d18ce49/server/src/main/java/org/opensearch/index/remote/RemoteSegmentTransferTracker.java#L262) which indirectly makes call to `System.nanoTime()`. This commit loosens the assumption that the test code execution will finish within +/-20ms. Instead it only assumes that the execution time span won't be shorter than predefined (and controlled) thread sleep interval and any larger interval value is considered a success. The whole point of this test is not to verify execution speed with defined precision. Instead the point is that the [`getTimeMsLag()`](https://github.com/opensearch-project/OpenSearch/blob/2b17902643738f0d2a75ade7c85cbca94d18ce49/server/src/main/java/org/opensearch/index/remote/RemoteSegmentTransferTracker.java#L262) method returns either 0 (for specific conditions) or possitive number (assuming that `remoteRefreshStartTimeMs` is not greater than `System.nanoTime()`). Closes: opensearch-project#14325 Signed-off-by: Lukáš Vlček <[email protected]>
1 parent b6c80b1 commit d1cc5c7

File tree

2 files changed

+9
-8
lines changed

2 files changed

+9
-8
lines changed

server/src/main/java/org/opensearch/index/remote/RemoteSegmentTransferTracker.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public class RemoteSegmentTransferTracker extends RemoteTransferTracker {
6565
private volatile long remoteRefreshSeqNo;
6666

6767
/**
68-
* The refresh time of most recent remote refresh.
68+
* The refresh time of the most recent remote refresh.
6969
*/
7070
private volatile long remoteRefreshTimeMs;
7171

@@ -76,7 +76,7 @@ public class RemoteSegmentTransferTracker extends RemoteTransferTracker {
7676
private volatile long remoteRefreshStartTimeMs = -1;
7777

7878
/**
79-
* The refresh time(clock) of most recent remote refresh.
79+
* The refresh time(clock) of the most recent remote refresh.
8080
*/
8181
private volatile long remoteRefreshClockTimeMs;
8282

server/src/test/java/org/opensearch/index/remote/RemoteSegmentTransferTrackerTests.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,16 @@ public void testComputeTimeLagOnUpdate() throws InterruptedException {
152152
transferTracker.updateLocalRefreshTimeMs(currentTimeMsUsingSystemNanos());
153153

154154
transferTracker.updateLatestLocalFileNameLengthMap(List.of("test"), k -> 1L);
155-
// Sleep for 100ms and then the lag should be within 100ms +/- 20ms
156-
Thread.sleep(100);
157-
assertTrue(Math.abs(transferTracker.getTimeMsLag() - 100) <= 20);
155+
// Sleep for 100ms and then the lag should not be shorter
156+
long span = 100;
157+
Thread.sleep(span);
158+
assertTrue("Lag is not expected to be shorter than span [" + span + "ms]", transferTracker.getTimeMsLag() >= span);
158159

159160
transferTracker.updateRemoteRefreshTimeMs(transferTracker.getLocalRefreshTimeMs());
160161
transferTracker.updateLocalRefreshTimeMs(currentTimeMsUsingSystemNanos());
161-
long random = randomIntBetween(50, 200);
162-
Thread.sleep(random);
163-
assertTrue(Math.abs(transferTracker.getTimeMsLag() - random) <= 20);
162+
long randomSpan = randomIntBetween(50, 200);
163+
Thread.sleep(randomSpan);
164+
assertTrue("Lag is not expected to be shorter than span [" + randomSpan + "ms]", transferTracker.getTimeMsLag() >= randomSpan);
164165
}
165166

166167
public void testAddUploadBytesStarted() {

0 commit comments

Comments
 (0)