KAFKA-14247: Define event handler interface and events by philipnee · Pull Request #12663 · apache/kafka

philipnee · 2022-09-19T21:29:38Z

https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor

We want to abstract away the queues from the new KafkaConsumer using an EventHandler.

poll: Retrieve a response from the eventHandler
add: Add a request to the handler

In the subsequent PRs, I'll add a backgroundThread inside of the eventHandler to actually consume the events.

*Unit tests

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

philipnee · 2022-09-19T23:47:59Z

cc @guozhangwang @hachikuji @dajac for review.

philipnee · 2022-09-21T16:56:44Z

Thanks @hachikuji and @guozhangwang - I just updated the PR. I'm settling with ApplicationEvent and BackgroundEvent. Also, brief definitions for the abstract classes have been added.

guozhangwang · 2022-09-21T17:05:33Z

Thanks @philipnee , I think we can merge the PR as is since it's new interfaces and if we found anything in follow-up developments we can always come back and change them. So LGTM.

philipnee · 2022-09-21T17:17:28Z

Thanks @guozhangwang! Agreed, we can make incremental changes to these implementation as we make more progress.

Documentation on the beahvior

hachikuji · 2022-09-26T20:19:04Z

+ * This class interfaces with the KafkaConsumer and the background thread. It allows the caller to enqueue events via
+ * the {@code add()} method and to retrieve events via the {@code poll()} method.
+ */
+public interface EventHandler {


I guess this is for the application side since we add application events and poll background events. Are we going to need something similar for the background thread as well?

I don't think we need another handler on the background thread, what I'm thinking is to have the background thread living inside of the handler implementation, process the application events, and produce the background events.

Stubbed event handler

This reverts commit 625b16c.

This reverts commit 816b285.

clean up documentation implemented poll and commitSync clean up clean up

hachikuji · 2022-09-28T19:38:55Z

+                    }
+                }
+
+                maybeSendFetches(); // send new fetches


I would think this would be handled by the background thread.

the intention was to enqueue a fetch event, I think we still want the poll loop to drive the fetching, right? Maybe a better naming is needed. enqueueFetchEvent() perhaps?

Thanks, @hachikuji - removed this line per your comment.

hachikuji · 2022-09-30T21:24:48Z

+import java.util.concurrent.TimeoutException;
+
+/**
+ * This is the prototype of the consumer that uses the {@link EventHandler} to process application events.


I think the main thing this doc should mention is that all network IO will be done in a background thread instead of being partially driven by the application calling poll(). Also, perhaps it would be helpful to link to the wiki so that people can understand a little better?

hachikuji

Thanks, LGTM. Hopefully this is the beginning of the end of the single-threaded consumer.

Adds skeleton of the background thread. 1-pager: https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor Continuation of #12663 Reviewers: Guozhang Wang <guozhang@apache.org>, Kirk True <kirk@mustardgrain.com>, John Roesler <vvcephei@apache.org>

Add some initial interfaces to kick off https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor. We introduce an `EventHandler` interface and a new consumer implementation which demonstrates how it will be used. Subsequent PRs will continue to flesh out the implementation. Reviewers: Guozhang Wang wangguoz@gmail.com, Jason Gustafson <jason@confluent.io>

…2672) Adds skeleton of the background thread. 1-pager: https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor Continuation of apache#12663 Reviewers: Guozhang Wang <guozhang@apache.org>, Kirk True <kirk@mustardgrain.com>, John Roesler <vvcephei@apache.org>

Add some initial interfaces to kick off https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor. We introduce an `EventHandler` interface and a new consumer implementation which demonstrates how it will be used. Subsequent PRs will continue to flesh out the implementation. Reviewers: Guozhang Wang wangguoz@gmail.com, Jason Gustafson <jason@confluent.io>

…#30) Add some initial interfaces to kick off https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor. We introduce an `EventHandler` interface and a new consumer implementation which demonstrates how it will be used. Subsequent PRs will continue to flesh out the implementation. Reviewers: Guozhang Wang wangguoz@gmail.com, Jason Gustafson <jason@confluent.io> Co-authored-by: Philip Nee <pnee@confluent.io>

…2672) Adds skeleton of the background thread. 1-pager: https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor Continuation of apache#12663 Reviewers: Guozhang Wang <guozhang@apache.org>, Kirk True <kirk@mustardgrain.com>, John Roesler <vvcephei@apache.org>

…2672) (#76) Adds skeleton of the background thread. 1-pager: https://cwiki.apache.org/confluence/display/KAFKA/Proposal%3A+Consumer+Threading+Model+Refactor Continuation of apache#12663 Reviewers: Guozhang Wang <guozhang@apache.org>, Kirk True <kirk@mustardgrain.com>, John Roesler <vvcephei@apache.org> Co-authored-by: Philip Nee <pnee@confluent.io>

Philip Nee added 2 commits September 19, 2022 14:12

Basic event handler definition

6ce7fef

Define EventHandler interface

1597d31

philipnee marked this pull request as ready for review September 19, 2022 21:30

Philip Nee added 2 commits September 19, 2022 14:42

lint error

f60d2e2

moving packages

a1cd844

reduce the scope

fbcef3b

hachikuji reviewed Sep 20, 2022

View reviewed changes

guozhangwang reviewed Sep 20, 2022

View reviewed changes

Comment thread clients/src/main/java/org/apache/kafka/clients/consumer/internals/EventHandler.java Outdated

Comment thread ...s/src/main/java/org/apache/kafka/clients/consumer/internals/events/ConsumerRequestEvent.java Outdated

PR comments for naming and documentation

3ced858

philipnee changed the title ~~[Consumer Refactor] Define event handler interface and events~~ KAFKA-14247: Define event handler interface and events Sep 21, 2022

hachikuji reviewed Sep 21, 2022

View reviewed changes

Comment thread clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/EventHandler.java Outdated

Documentation on the beahvior

ccb5f14

Documentation on the beahvior

philipnee force-pushed the consumer-refactor-event-handler-definition branch from 1c7be91 to ccb5f14 Compare September 21, 2022 19:27

Philip Nee added 2 commits September 21, 2022 13:09

Add isEmpty and remove excessive public

b04d33d

Remove unused import

8489337

philipnee mentioned this pull request Sep 21, 2022

KAFKA-14247: Consumer background thread base implementation #12672

Merged

3 tasks

Philip Nee added 2 commits September 21, 2022 13:40

Handle capacity limitation

6629e27

clean up a comment

6d97d2a

philipnee requested a review from hachikuji September 23, 2022 22:38

hachikuji reviewed Sep 26, 2022

View reviewed changes

A stubbed event handler to demonstrate the usage.

816b285

Stubbed event handler

philipnee force-pushed the consumer-refactor-event-handler-definition branch from a45fb02 to 816b285 Compare September 26, 2022 22:59

Philip Nee added 4 commits September 26, 2022 16:04

typo

625b16c

Revert "typo"

078ab7f

This reverts commit 625b16c.

Revert "A stubbed event handler to demonstrate the usage."

3213e57

This reverts commit 816b285.

Prototyping

c6d67e2

clean up and documentation

f5b25e6

clean up documentation implemented poll and commitSync clean up clean up

philipnee force-pushed the consumer-refactor-event-handler-definition branch from 83b6190 to f5b25e6 Compare September 27, 2022 20:58

hachikuji reviewed Sep 28, 2022

View reviewed changes

Philip Nee added 6 commits September 28, 2022 13:08

fetches should happen on the background thread.

43f798c

fix a lint error

ffe6432

clarify poll loop

bf73d8f

improve documentation

d77e1e3

more documentation

d3d3d54

clarification

9cb4682

hachikuji reviewed Sep 30, 2022

View reviewed changes

Comment thread ...rc/main/java/org/apache/kafka/clients/consumer/internals/AbstractPrototypeAsyncConsumer.java Outdated

Philip Nee added 2 commits September 30, 2022 15:21

documentation

e0949e9

typo: 'prototype

e63f1ee

hachikuji approved these changes Sep 30, 2022

View reviewed changes

hachikuji merged commit c3690a3 into apache:trunk Oct 3, 2022

philipnee added KIP-848 The Next Generation of the Consumer Rebalance Protocol ctr Consumer Threading Refactor (KIP-848) labels May 23, 2023

rutvijmehta-harness mentioned this pull request Feb 9, 2024

KAFKA-14247: Consumer background thread base implementation (#12672) rutvijmehta-harness/kafka#76

Merged

Uh oh!

Conversation

philipnee commented Sep 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

philipnee commented Sep 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipnee commented Sep 21, 2022

Uh oh!

guozhangwang commented Sep 21, 2022

Uh oh!

philipnee commented Sep 21, 2022

Uh oh!

Uh oh!

hachikuji Sep 26, 2022

Choose a reason for hiding this comment

Uh oh!

philipnee Sep 26, 2022

Choose a reason for hiding this comment

Uh oh!

hachikuji Sep 28, 2022

Choose a reason for hiding this comment

Uh oh!

philipnee Sep 28, 2022

Choose a reason for hiding this comment

Uh oh!

philipnee Sep 28, 2022

Choose a reason for hiding this comment

Uh oh!

hachikuji Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hachikuji left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

philipnee commented Sep 19, 2022 •

edited

Loading