lianetm commented on PR #20324:
URL: https://github.com/apache/kafka/pull/20324#issuecomment-3255268654
> Given the above, can we introduce DoEverythingNeededForFetchEvent as an
optimized "fast path" that can execute the bulk of fetch uninterrupted in the
background thread, but still have a "slow path" for when we need to fall back
to using one or both of PollEvent or CheckAndUpdatePositionsEvent, when needed?
Well, I was hoping we could move to a simplified fast path only,
consolidating the fetch logic in the background. Single event to trigger fetch
(including all the pre-fetch actions, and triggering fetch), and keeping
callbacks triggering in the app thread. This is what I had in mind to explore:
- Poll event -> remove and consolidate within `CreateFetchRequestEvent` (to
trigger reconciliation and auto-commit before updating positions to fetch)
- updateAssignmentMetadataIfNeeded -> extract what needs to happen in the
app thread and leave it there. Consolidate fetch-related logic within
`CreateFetchRequestEvent`
- Commit callbacks: remain in app thread, triggered via
`offsetCommitCallbackInvoker`
- `UpdatePatternSubscriptionEvent`: this logic simply needs to run
before joining a group, and we already have a hook for that in the background
(`membershipMgr.onConsumerPoll`), so we could consider removing this event and
move the logic there.
- Rebalance callbacks: remain in app thread, triggered via
`processBackgroundEvents` (this is unrelated to the previous
`UpdatePatternSubscriptionEvent` logic so no concerns with the order here I
would say)
- updateFetchPositions: consolidate within `CreateFetchRequestEvent`
Would that work? I'm still thinking about it myself, but if it works, it
would be a nice simplification to the current poll(), consolidating fetch in
the background with a single `DoEverythingNeededForFetchEvent`/`TriggerFetch`
event
Then, regarding:
> how to handle the CreateFetchRequestsEvent. The
FetchCollector.collectFetch() logic runs on the application thread
I don't quite get the concern, probably missing something? We are just
changing how we prep before sending a fetch (consolidating in a single event),
but no changes on how we wait for the `CreateFetchRequestsEvent` (we will
continue to wait for it), and no changes on how we take data out of the buffer
(collectFetch, in the app thread, etc, agreed).
--
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
