Skip to content

Commit 025c303

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 <lukas.vlcek@aiven.io>
1 parent 3b358e3 commit 025c303

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

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

Lines changed: 2 additions & 2 deletions
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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ public void testComputeTimeLagOnUpdate() throws InterruptedException {
154154
transferTracker.updateLatestLocalFileNameLengthMap(List.of("test"), k -> 1L);
155155
// Sleep for 100ms and then the lag should be within 100ms +/- 20ms
156156
Thread.sleep(100);
157-
assertTrue(Math.abs(transferTracker.getTimeMsLag() - 100) <= 20);
157+
assertTrue(transferTracker.getTimeMsLag() >= 100);
158158

159159
transferTracker.updateRemoteRefreshTimeMs(transferTracker.getLocalRefreshTimeMs());
160160
transferTracker.updateLocalRefreshTimeMs(currentTimeMsUsingSystemNanos());
161161
long random = randomIntBetween(50, 200);
162162
Thread.sleep(random);
163-
assertTrue(Math.abs(transferTracker.getTimeMsLag() - random) <= 20);
163+
assertTrue(transferTracker.getTimeMsLag() >= random);
164164
}
165165

166166
public void testAddUploadBytesStarted() {

0 commit comments

Comments
 (0)