lianetm commented on code in PR #16885:
URL: https://github.com/apache/kafka/pull/16885#discussion_r1724973566
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java:
##########
@@ -52,15 +59,27 @@ public class ApplicationEventProcessor implements
EventProcessor<ApplicationEven
private final ConsumerMetadata metadata;
private final SubscriptionState subscriptions;
private final RequestManagers requestManagers;
+ private final Time time;
+
+ /**
+ * OffsetFetch request triggered to update fetch positions. The request is
kept. It will be
+ * cleared every time a response with the committed offsets is received
and used to update
+ * fetch positions. If the response cannot be used because the
UpdateFetchPositions expired,
+ * it will be kept to be used on the next attempt to update fetch
positions if partitions
+ * remain the same.
+ */
+ private FetchCommittedOffsetsEvent pendingOffsetFetchEvent;
Review Comment:
Yeap, totally agree. I debated on that too. It would be a new
`UpdatePositionsEventProcessor` that still "dispatches events to their
corresponding managers", but agree that in this case it requires more logic to
achieve that.
I didn't go ahead with a separate class since the beginning just to see how
others felt about this. I saw this options:
1. leave it in the same processor where all events are indeed processed
2. have a new `UpdatePositionsEventProcessor` (just to leave it no specific
manager, and mix all the ones needed here)
3. have it in the offsetsRequestManager, given that most of the logic to
update positions is about partition offsets, mixing in a bit of committed
offsets.
I do like the 3rd one, mainly because I find it consistent with the
MembershipManager, that does what it needs and may require the
commitRequestManager. I felt I could be biased here of course :), but
@AndrewJSchofield also suggested this option 3, so if it makes sense to all I
go for it
--
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
