jolshan commented on code in PR #15432:
URL: https://github.com/apache/kafka/pull/15432#discussion_r1503408163
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java:
##########
@@ -1731,15 +1735,19 @@ public void onNewMetadataImage(
// Push an event for each coordinator.
coordinators.keySet().forEach(tp -> {
scheduleInternalOperation("UpdateImage(tp=" + tp + ", offset=" +
newImage.offset() + ")", tp, () -> {
- withContextOrThrow(tp, context -> {
- if (context.state == CoordinatorState.ACTIVE) {
+ CoordinatorContext context = coordinators.get(tp);
+ if (context != null && context.state ==
CoordinatorState.ACTIVE) {
Review Comment:
I noticed there are still some places we don't want the context to be null
-- ie loading the partition. (And for all the run methods we want the
coordinator to be active) Is the general rationale for this change that not
having the conetext/active state is ok if we expect the coordinator moved? Is
it possible we could try to load and the coordinator moves again? Just trying
to understand the cases we accept null or non-active coordinators vs when we
throw errors.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
