lucliu1108 commented on code in PR #23566:
URL: https://github.com/apache/kafka/pull/23566#discussion_r4100066681
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/StreamsGroupTopologyDescriptionManager.java:
##########
@@ -134,22 +212,25 @@ public void startCleanupCycle(
scheduleNextTick(timer, cleanupCheckIntervalMs, cycleSupplier);
}
- /**
- * Stop the cleanup cycle and release plugin-side resources. Flips {@code
running}
- * false (so the next timer tick refuses to fire) and cancels the
currently-scheduled
- * tick, then closes the plugin. Called by {@code
GroupCoordinatorService.shutdown}
- * before the runtime is closed, so writes already scheduled by the
previous tick
- * drain through their own futures rather than racing the runtime
tear-down.
- */
- @Override
- public void close() throws Exception {
+ /** Stops the cleanup cycle without touching the plugin; see {@link
#close}. */
+ public void stopCleanupCycle() {
if (running.compareAndSet(true, false)) {
TimerTask snapshot = scheduledTask;
if (snapshot != null) {
snapshot.cancel();
}
}
- if (plugin.isPresent()) {
+ }
+
+ /**
+ * Stops the cleanup cycle and closes the plugin. Called by
+ * {@code GroupCoordinatorService.shutdown} after {@code isActive} flips
false, since request
+ * handlers gate on {@code isActive} once and don't re-check it before
reaching the plugin.
+ */
+ @Override
+ public void close() throws Exception {
+ stopCleanupCycle();
+ if (closed.compareAndSet(false, true) && plugin.isPresent()) {
Review Comment:
Added 3 new tests:
- `testCloseInvokesPluginCloseOnlyOnce`: closes manager twice, each time the
`plugin.close()` is triggered it increment a counter. Validate that counter is
1.
- `testStopCleanupCycleNoOpWhenNotRunning`: don't spin up the manager and
call `manager#stopCleanupCycle()` , verify the manager is not running and no
scheduledCleanupTask available.
- `testStopCleanupCycleCancelsScheduledTask`: spin up the manager, validate
there's `scheduledCleanupTask` running. Then call `stopCleanupCycle`, verify
that the manager isn't running and the cleanupTask is cancelled.
--
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]
