Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplification of consumer internals #903

Open
LMnet opened this issue Mar 14, 2022 · 3 comments
Open

Simplification of consumer internals #903

LMnet opened this issue Mar 14, 2022 · 3 comments

Comments

@LMnet
Copy link
Member

LMnet commented Mar 14, 2022

From here:

At the current moment, consumer internals are pretty complex: we have a KafkaConsumer itself, a hand-maded KafkaConsumerActor, a queue for messages. Also, there is some complexity with different commit methods. And most of this complexity is to make all calls to a java consumer linearized. But actually, to achieve this we could use a simple Semaphore(1). Under the hood, this semaphore uses a queue (like an actor), but all low-level queue-related machinery is hidden. With this, we could dramatically reduce the complexity of library internals and make it more contributor-friendly.

@LMnet LMnet mentioned this issue Mar 14, 2022
@bplommer
Copy link
Member

That sounds good - the only thing I wonder is how we would handle prioritising client requests over polls? I wonder if instead of switching to cats.effect.Semaphore we should instead/first replace the different kind of actor requests with a single semaphore-like permit request

@bplommer
Copy link
Member

I'm thinking something like #906 - does that fit with what you had in mind?

@aartigao
Copy link
Contributor

I agree with @LMnet that current code is not contributor-friendly. That's my experience at least 😅 I find it difficult to follow and I'm pretty confident that we can simplify it by removing completely KafkaConsumerActor.

I'm working on a POC to simplify the internals (let's see how far I can get).

About how to prioritize client requests over polls: wouldn't Priority Queue be the answer to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants