dajac commented on code in PR #14849:
URL: https://github.com/apache/kafka/pull/14849#discussion_r1409265390
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java:
##########
@@ -1315,7 +1329,10 @@ public void scheduleLoadOperation(
case FAILED:
case INITIAL:
context.transitionTo(CoordinatorState.LOADING);
- loader.load(tp,
context.coordinator).whenComplete((summary, exception) -> {
+ loader.load(
+ tp,
+ context
Review Comment:
I was thinking about the actual impact of passing the context to another
thread. One issue here is that the context is protected by a lock and the
loader thread does not use it at all. One potential issue for instance is that
the context could transition to Closed while the coordinator is loaded.
Therefore, I wonder whether we should structure things a bit differently. I
am thinking about introducing a new class which would hold the coordinator,
snapshot registry, lastWrittenOffset and lastCommittedOffset. The class would
also implement the CoordinatorPlayback interface. In the context, we would
remove all those fields and replace them by a reference to an instance of that
new class. I think that we would have to also synchronize the methods to ensure
a memory barrier. What do you think?
--
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
