Skip to content

Commit a2ea1e1

Browse files
committed
kvm,utils/test,config/test: fix flaky unit tests
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent b99a030 commit a2ea1e1

File tree

4 files changed

+50
-15
lines changed

4 files changed

+50
-15
lines changed

framework/config/src/test/java/org/apache/cloudstack/framework/config/ConfigKeyScheduledExecutionWrapperTest.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import java.util.concurrent.Executors;
2626
import java.util.concurrent.ScheduledExecutorService;
2727
import java.util.concurrent.TimeUnit;
28+
import java.util.concurrent.atomic.AtomicInteger;
29+
import java.util.concurrent.atomic.AtomicLong;
2830

2931
import static org.hamcrest.MatcherAssert.assertThat;
3032
import static org.hamcrest.Matchers.is;
@@ -35,6 +37,8 @@
3537
public class ConfigKeyScheduledExecutionWrapperTest {
3638
private final ScheduledExecutorService executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("TestExecutor"));
3739

40+
protected static final int DELTA_WAIT_MS = 100;
41+
3842
@Mock
3943
ConfigKey<Integer> configKey;
4044

@@ -76,7 +80,7 @@ public void scheduleOncePerSecondTest() {
7680

7781
private void waitSeconds(int seconds) {
7882
try {
79-
Thread.sleep(seconds * 1000L + 100);
83+
Thread.sleep(seconds * 1000L + DELTA_WAIT_MS);
8084
} catch (InterruptedException e) {
8185
throw new RuntimeException(e);
8286
}
@@ -144,8 +148,9 @@ public void temporaryDisableRunsTest() {
144148
}
145149

146150
static class TestRunnable implements Runnable {
147-
private Integer runCount = 0;
151+
private final AtomicInteger runCount = new AtomicInteger(0);
148152
private int waitSeconds = 0;
153+
private final AtomicLong resetMs = new AtomicLong(0);
149154

150155
TestRunnable(int waitSeconds) {
151156
this.waitSeconds = waitSeconds;
@@ -156,7 +161,11 @@ static class TestRunnable implements Runnable {
156161

157162
@Override
158163
public void run() {
159-
runCount++;
164+
long resetMsVal = resetMs.get();
165+
if (resetMsVal == 0 || System.currentTimeMillis() - resetMsVal > DELTA_WAIT_MS) {
166+
runCount.incrementAndGet();
167+
}
168+
resetMs.set(0);
160169
if (waitSeconds > 0) {
161170
try {
162171
Thread.sleep(waitSeconds * 1000L);
@@ -167,11 +176,12 @@ public void run() {
167176
}
168177

169178
public int getRunCount() {
170-
return this.runCount;
179+
return this.runCount.get();
171180
}
172181

173182
public void resetRunCount() {
174-
this.runCount = 0;
183+
this.runCount.set(0);
184+
this.resetMs.set(System.currentTimeMillis());
175185
}
176186
}
177187
}

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6396,18 +6396,41 @@ public void disconnected() {
63966396
for (DisconnectHook hook : _disconnectHooks) {
63976397
hook.start();
63986398
}
6399+
// compute a shared deadline = now + max(timeout of hooks)
63996400
long start = System.currentTimeMillis();
6401+
long maxTimeoutMs = 0;
6402+
for (DisconnectHook hook : _disconnectHooks) {
6403+
maxTimeoutMs = Math.max(maxTimeoutMs, hook.getTimeoutMs());
6404+
}
6405+
System.out.println("Max timeout for disconnect hooks is " + maxTimeoutMs + "ms");
6406+
final long globalDeadline = start + maxTimeoutMs;
6407+
6408+
// join each hook using remaining time until the shared deadline
64006409
for (DisconnectHook hook : _disconnectHooks) {
64016410
try {
6402-
long elapsed = System.currentTimeMillis() - start;
6403-
long remaining = hook.getTimeoutMs() - elapsed;
6404-
long joinWait = remaining > 0 ? remaining : 1;
6405-
hook.join(joinWait);
6406-
hook.interrupt();
6411+
long now = System.currentTimeMillis();
6412+
long hookDeadline = start + Math.max(0, hook.getTimeoutMs());
6413+
long effectiveDeadline = Math.min(globalDeadline, hookDeadline);
6414+
long remaining = effectiveDeadline - now;
6415+
System.out.println("Joining disconnect hook " + hook + ", remaining time is " + remaining + "ms");
6416+
if (remaining <= 0) {
6417+
// overall timeout already expired
6418+
if (hook.isAlive()) {
6419+
System.out.println("Interrupting disconnect hook " + hook + " due to timeout");
6420+
hook.interrupt();
6421+
}
6422+
continue;
6423+
}
6424+
hook.join(remaining);
6425+
if (hook.isAlive()) {
6426+
hook.interrupt();
6427+
}
64076428
} catch (InterruptedException ex) {
6408-
LOGGER.warn("Interrupted disconnect hook: " + ex.getMessage());
6429+
Thread.currentThread().interrupt();
6430+
LOGGER.warn("Interrupted while waiting for disconnect hook: " + ex.getMessage());
64096431
}
64106432
}
6433+
64116434
_disconnectHooks.clear();
64126435
}
64136436

server/src/test/java/org/apache/cloudstack/vm/schedule/VMSchedulerImplTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,9 @@ public void testScheduleNextJobScheduleFutureScheduleWithTimeZoneChecks() throws
247247

248248
@Test
249249
public void testScheduleNextJobScheduleCurrentSchedule() {
250-
Date now = DateUtils.setSeconds(new Date(), 0);
251-
Date expectedScheduledTime = DateUtils.round(DateUtils.addMinutes(now, 1), Calendar.MINUTE);
250+
Date now = DateUtils.round(new Date(), Calendar.MINUTE);
251+
Date expectedScheduledTime = DateUtils.addMinutes(now, 1);
252+
252253
UserVm vm = Mockito.mock(UserVm.class);
253254

254255
VMScheduleVO vmSchedule = Mockito.mock(VMScheduleVO.class);
@@ -257,7 +258,8 @@ public void testScheduleNextJobScheduleCurrentSchedule() {
257258
Mockito.when(vmSchedule.getTimeZoneId()).thenReturn(TimeZone.getTimeZone("UTC").toZoneId());
258259
Mockito.when(vmSchedule.getStartDate()).thenReturn(DateUtils.addDays(now, -1));
259260
Mockito.when(userVmManager.getUserVm(Mockito.anyLong())).thenReturn(vm);
260-
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
261+
262+
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, now);
261263

262264
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
263265
}

utils/src/test/java/org/apache/cloudstack/utils/cache/LazyCacheTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
@RunWith(MockitoJUnitRunner.class)
3232
public class LazyCacheTest {
33-
private final long expireSeconds = 1;
33+
private final long expireSeconds = 2;
3434
private final String cacheValuePrefix = "ComputedValueFor:";
3535
private LazyCache<String, String> cache;
3636
private Function<String, String> mockLoader;

0 commit comments

Comments
 (0)