Skip to content

KAFKA-12388: Share broker channel between alterIsrManager and lifecycleManager#10255

Closed
dengziming wants to merge 1 commit intoapache:trunkfrom
dengziming:KAFKA-12388-share-broker-channel
Closed

KAFKA-12388: Share broker channel between alterIsrManager and lifecycleManager#10255
dengziming wants to merge 1 commit intoapache:trunkfrom
dengziming:KAFKA-12388-share-broker-channel

Conversation

@dengziming
Copy link
Copy Markdown
Member

@dengziming dengziming commented Mar 4, 2021

More detailed description of your change
There are some BorkerToControllerChannerManager in BrokerServer and KafkaServer,
We are planning to consolidate into two channels eventually:

  1. broker to controller channel
  2. client to controller channel

Auto topic creation and forwarding fall into the 2nd category, while AlterIsr, lifecycleManager, and logDirEventManager(see KAFKA-9837) would be the 1st category.
#10135 is consolidating the 2nd category, this pr is trying to consolidate the 1st category.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

@dengziming dengziming force-pushed the KAFKA-12388-share-broker-channel branch from c2d1496 to 189aa3e Compare March 4, 2021 02:54
@dengziming dengziming force-pushed the KAFKA-12388-share-broker-channel branch from 189aa3e to cc2bfb0 Compare March 16, 2021 02:08
@dengziming
Copy link
Copy Markdown
Member Author

@abbccdda @mumrah @hachikuji to have a look.😉

@dengziming dengziming force-pushed the KAFKA-12388-share-broker-channel branch from cc2bfb0 to bfc99c3 Compare March 19, 2021 08:16
@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented May 19, 2021

Thanks for looking at this. The issue that I see with sharing channels (really sockets) in this way is that we don't want the broker heartbeat to get delayed by something else. If the heartbeat gets delayed too long, the consequences could be really bad.

I think what we should do here is get rid of head-of-line blocking for Kafka RPCs, so that they can be done in parallel. Really, the main reason why we still have head-of-line blocking is because of the difficulty of moving the producer and consumer off of that model. So we could get rid of HOL blocking for all requests except produce and consume without too many changes to the code, I think.

I think we need to fix the HOL blocking problem before we start consolidating channels, to avoid regressions in functionality. cc @ijuma @hachikuji @abbccdda

@dengziming dengziming closed this Nov 11, 2023
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.

2 participants