Skip to content

Commit 1bd9057

Browse files
Add releaseOnCompletion hook to OperationalJob
1 parent 509b15b commit 1bd9057

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

server/src/main/java/org/apache/cassandra/sidecar/job/OperationalJob.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,24 @@ public boolean requiresCoordination()
183183
return false;
184184
}
185185

186+
/**
187+
* Whether the manager should release the active operation lock (via
188+
* {@link OperationalJobCoordinator#clearActive}) when this job completes locally. Only consulted for jobs that
189+
* {@link #requiresCoordination() require coordination}.
190+
* <p>
191+
* Single-node coordinated jobs return {@code true} (the default): the Sidecar that acquires the lock also
192+
* finishes the work, so releasing on local completion is correct. Distributed cluster-wide jobs whose work
193+
* finishes on other nodes return {@code false} and rely on the orchestration layer to call
194+
* {@link OperationalJobCoordinator#clearActive} once all nodes reach a terminal state.
195+
*
196+
* @return {@code true} if the manager should release the lock on local completion; {@code false} to defer
197+
* release to the orchestration layer
198+
*/
199+
public boolean releasesOnCompletion()
200+
{
201+
return true;
202+
}
203+
186204
@Override
187205
public final Void result()
188206
{

server/src/main/java/org/apache/cassandra/sidecar/job/OperationalJobManager.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,12 @@ public void trySubmitJob(OperationalJob job,
127127
{
128128
if (ar.succeeded() && Boolean.TRUE.equals(ar.result()))
129129
{
130-
job.asyncResult().onComplete(result -> releaseActiveOperationLock(job));
130+
// Distributed jobs that finish on other nodes opt out of auto-release; the orchestration
131+
// layer clears the lock once all nodes reach a terminal state.
132+
if (job.releasesOnCompletion())
133+
{
134+
job.asyncResult().onComplete(result -> releaseActiveOperationLock(job));
135+
}
131136
trackAndExecute(job, onComplete, serviceExecutorPool, waitTime);
132137
}
133138
else

server/src/test/java/org/apache/cassandra/sidecar/job/OperationalJobManagerTest.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static org.apache.cassandra.sidecar.common.data.OperationalJobStatus.SUCCEEDED;
4646
import static org.assertj.core.api.Assertions.assertThat;
4747
import static org.mockito.ArgumentMatchers.any;
48+
import static org.mockito.Mockito.after;
4849
import static org.mockito.Mockito.mock;
4950
import static org.mockito.Mockito.never;
5051
import static org.mockito.Mockito.timeout;
@@ -253,6 +254,29 @@ void testCoordinationFailsWhenCoordinationDisabled() throws InterruptedException
253254
assertThat(tracker.inflightJobsByOperation(job.name())).doesNotContain(job);
254255
}
255256

257+
@Test
258+
void testLockNotReleasedWhenJobOptsOut() throws InterruptedException
259+
{
260+
OperationalJobTracker tracker = new InMemoryOperationalJobTracker(4);
261+
OperationalJobCoordinator coordinator = mock(OperationalJobCoordinator.class);
262+
when(coordinator.trySetActive(any(), any())).thenReturn(true);
263+
OperationalJobManager manager = new OperationalJobManager(tracker, coordinator, executorPool);
264+
CountDownLatch latch = new CountDownLatch(1);
265+
266+
// A distributed cluster-wide job that acquires the lock locally but relies on the orchestration
267+
// layer to clear it once all nodes finish, so the manager must not auto-release on local completion.
268+
OperationalJob job = createNonReleasingCoordinatedJob(UUIDs.timeBased());
269+
BiConsumer<OperationalJob, OperationalJobConflictException> onComplete = (j, ex) -> {
270+
assertThat(ex).isNull();
271+
latch.countDown();
272+
};
273+
274+
manager.trySubmitJob(job, onComplete, executorPool.service(), SecondBoundConfiguration.parse("5s"));
275+
assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue();
276+
verify(coordinator).trySetActive(OperationType.MOVE, job.jobId());
277+
verify(coordinator, after(1000).never()).clearActive(any(), any());
278+
}
279+
256280
@Test
257281
void testCoordinatorNotCalledWhenJobDoesNotRequireCoordination() throws InterruptedException
258282
{
@@ -301,4 +325,40 @@ protected Future<Void> executeInternal()
301325
}
302326
};
303327
}
328+
329+
private static OperationalJob createNonReleasingCoordinatedJob(UUID jobId)
330+
{
331+
return new OperationalJob(jobId)
332+
{
333+
@Override
334+
public boolean hasConflict(@NotNull List<OperationalJob> sameOperationJobs)
335+
{
336+
return false;
337+
}
338+
339+
@Override
340+
public OperationType operationType()
341+
{
342+
return OperationType.MOVE;
343+
}
344+
345+
@Override
346+
public boolean requiresCoordination()
347+
{
348+
return true;
349+
}
350+
351+
@Override
352+
public boolean releasesOnCompletion()
353+
{
354+
return false;
355+
}
356+
357+
@Override
358+
protected Future<Void> executeInternal()
359+
{
360+
return Future.succeededFuture();
361+
}
362+
};
363+
}
304364
}

0 commit comments

Comments
 (0)