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

UpdatePartitionState to avoid restarting Producer #375

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

suyashtava
Copy link

@suyashtava suyashtava commented Sep 14, 2022

On Every new Partition, We are killing the whole Producer Service, and restarting it, instead we can update the state of Partition to include new partitions. This will stop unnecessary restart.

Issue: #376

UpdatePartitionState and call it from NewPartitionHandler to avoid restarting Producer service again in case of a new partition
@suyashtava
Copy link
Author

suyashtava commented Sep 15, 2022

@linkedin, @mhratson, = @andrewchoi5 @Lincong for review

@suyashtava
Copy link
Author

@efeg for review pls.

@suyashtava
Copy link
Author

@CCisGG Any one whom you can add for review please.

@CCisGG
Copy link

CCisGG commented Sep 27, 2022

I don't have much context on this repo. @mitchhh22 could you or your team help to review this?

@mhratson
Copy link
Collaborator

mhratson commented Sep 27, 2022 via email

@mhratson
Copy link
Collaborator

mhratson commented Oct 4, 2022

@suyashtava thanks for the contribution!
Before accepting the PR i'd like to understand more of the problem being solved by this.

Could you please describe why restarting is an issue? While I can assume that restarting may be slow i'd like to know other arguments as well if available.

Thanks

@suyashtava
Copy link
Author

@suyashtava thanks for the contribution! Before accepting the PR i'd like to understand more of the problem being solved by this.

Could you please describe why restarting is an issue? While I can assume that restarting may be slow i'd like to know other arguments as well if available.

Thanks

@mhratson

Background:
We were using KMF for Broker health Detection, and since all partitions use the same Producer, even if one broker is slow in a cluster all the other partitions in the queue of the same Broker will also get slow, and that was making it difficult to find an unhealthy broker.

For this, we introduced 1:1 mapping of Partition and Producer so other partitions can still be produced by a producer and not blocked by other Producer feeling slow.

Challenge:
Now on every shutdown, we need to close multiple Producers, which we did. But whenever a new partition was added, it again restarted all the producers which was causing significant slowness.

Proposal:
We can discuss on 1:1 Mapping on Producer: Partition but surely restarting all Threads on each new partition is an overhead even in the current KMF, when we can just simply add the new partition to the scheduler.

@suyashtava
Copy link
Author

suyashtava commented Oct 9, 2022

If the 1:1 mapping of Producer and Partition sounds good I can raise another PR for the same, after this one.
@mitchhh22 @mhratson @andrewchoi5 @Lincong @efeg

@suyashtava
Copy link
Author

@mhratson by any chance u get a moment to check this?
Thanks in advance.

@mhratson
Copy link
Collaborator

For this, we introduced 1:1 mapping of Partition and Producer so other partitions can still be produced by a producer and not blocked by other Producer feeling slow.

That's not the case for this kafka-monitor, isn't it?

@suyashtava
Copy link
Author

For this, we introduced 1:1 mapping of Partition and Producer so other partitions can still be produced by a producer and not blocked by other Producer feeling slow.

That's not the case for this kafka-monitor, isn't it?

@mhratson Apolgies, for some personal reason I had to drop system.
Reopening this thread. You are correct this is not the case for this KMF.

This opens 2 things: IMHO We should Decouple Producer for each partition here aswell, so that it would be easy to detect which broker is slow.

Even if we decide against above, atleast we should not be restarting whole Producer in case of new Partition, and it should be attached to same Producer, that way we will increase KMF Availability.

@suyashtava
Copy link
Author

Added a PR. #394 for Issue #395 (Multiple Pruder per partition)

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

Successfully merging this pull request may close these issues.

None yet

3 participants