Skip to content

Commit

Permalink
Update the Worker shutdown logic to make sure that the LeaseCleanup M…
Browse files Browse the repository at this point in the history
…anager also terminates all the threads that it has started (#817)

Co-authored-by: kmz <kmz@users.noreply.github.com>
  • Loading branch information
kmz and kmz authored Jul 1, 2021
1 parent f65b3fd commit a258356
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,10 @@ public void shutdown() {
// Lost leases will force Worker to begin shutdown process for all shard consumers in
// Worker.run().
leaseCoordinator.stop();

// Stop the lease cleanup manager
leaseCleanupManager.shutdown();

leaderElectedPeriodicShardSyncManager.stop();
workerStateChangeListener.onWorkerStateChange(WorkerStateChangeListener.WorkerState.SHUT_DOWN);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,22 @@ public void start() {
}
}

/**
* Stops the lease cleanup thread, which is scheduled periodically as specified by
* {@link LeaseCleanupManager#leaseCleanupIntervalMillis}
*/
public void shutdown() {
if (isRunning) {
log.info("Stopping the lease cleanup thread.");
completedLeaseStopwatch.stop();
garbageLeaseStopwatch.stop();
deletionThreadPool.shutdown();
isRunning = false;
} else {
log.info("Lease cleanup thread already stopped.");
}
}

/**
* Enqueues a lease for deletion without check for duplicate entry. Use {@link #isEnqueuedForDeletion}
* for checking the duplicate entries.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ public final void testSubsequentStarts() {
leaseCleanupManager.start();
}

/**
* Tests subsequent calls to shutdown {@link LeaseCleanupManager}.
*/
@Test
public final void testSubsequentShutdowns() {
leaseCleanupManager.start();
Assert.assertTrue(leaseCleanupManager.isRunning());
leaseCleanupManager.shutdown();
Assert.assertFalse(leaseCleanupManager.isRunning());
leaseCleanupManager.shutdown();
}

/**
* Tests that when both child shard leases are present, we are able to delete the parent shard for the completed
* shard case.
Expand Down

0 comments on commit a258356

Please sign in to comment.