yifan-c commented on code in PR #192:
URL: https://github.com/apache/cassandra-sidecar/pull/192#discussion_r1955281874
##########
server/src/test/java/org/apache/cassandra/sidecar/tasks/PeriodicTaskExecutorTest.java:
##########
@@ -162,80 +150,61 @@ void testUnscheduleShouldStopExecution()
@Test
void testUnscheduleNonExistTaskHasNoEffect()
{
- PeriodicTask notScheduled = createSimplePeriodicTask("simple task", 1,
() -> {
- });
+ PeriodicTask notScheduled = createSimplePeriodicTask("simple task", 1,
1, () -> {});
taskExecutor.unschedule(notScheduled);
- assertThat(taskExecutor.poisonPilledTasks()).isEmpty();
assertThat(taskExecutor.timerIds()).isEmpty();
}
@Test
- void testRescheduleNonExistTaskShouldNotClose()
+ void testUnscheduleNonExistTaskShouldNotClose()
{
AtomicBoolean isCloseCalled = new AtomicBoolean(false);
- CountDownLatch taskScheduled = new CountDownLatch(1);
- PeriodicTask task = new PeriodicTask()
- {
- @Override
- public DurationSpec delay()
- {
- return MillisecondBoundConfiguration.ONE;
- }
-
- @Override
- public void execute(Promise<Void> promise)
- {
- taskScheduled.countDown();
- promise.complete();
- }
+ PeriodicTask task = createSimplePeriodicTask("SimpleTask", 1, 1,
+ () ->
ScheduleDecision.EXECUTE,
+ () -> {},
+ () ->
isCloseCalled.set(true));
- @Override
- public void close()
- {
- isCloseCalled.set(true);
- }
- };
-
- // for such unscheduled task, reschedule has the same effect as
schedule.
- taskExecutor.reschedule(task);
- Uninterruptibles.awaitUninterruptibly(taskScheduled);
+ taskExecutor.unschedule(task);
assertThat(isCloseCalled.get())
.describedAs("When rescheduling an unscheduled task, the close method
of the task should not be called")
.isFalse();
}
@Test
- void testReschedule()
+ void testRescheduleDecision()
{
- AtomicInteger counter1 = new AtomicInteger(0);
- AtomicInteger counter2 = new AtomicInteger(0);
- int totalSamples = 10;
- Set<Integer> counterValues = ConcurrentHashMap.newKeySet(totalSamples);
-
- CountDownLatch latch1 = new CountDownLatch(1);
- // The periodic task increment counter1 initially, then switches to
increment counter 2 once it is rescheduled
- PeriodicTask task = createSimplePeriodicTask("simple periodic task",
1, () -> {
- if (latch1.getCount() > 0)
- {
- counter1.incrementAndGet();
- }
- else
- {
- counterValues.add(counter2.incrementAndGet());
- }
- Uninterruptibles.awaitUninterruptibly(latch1);
- });
+ testRescheduleDecision(1, 0);
+ testRescheduleDecision(0, 0);
+ testRescheduleDecision(1, 1);
+ testRescheduleDecision(0, 1);
+ }
+
+ void testRescheduleDecision(long initialDelay, long delay)
Review Comment:
This is the test that reproduce the bug.
Test fails with the earlier version of `PeriodicTaskExecutor`, as there are
flee tasks that keep on running.
Verify with the following 2 commands
```
git checkout upstream/trunk --
server/src/main/java/org/apache/cassandra/sidecar/tasks/PeriodicTaskExecutor.java
./gradlew :server:test --tests
org.apache.cassandra.sidecar.tasks.PeriodicTaskExecutorTest.testRescheduleDecision
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
