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

Reduce number of calls to Kafka #142

Closed
wants to merge 1 commit into from
Closed

Reduce number of calls to Kafka #142

wants to merge 1 commit into from

Conversation

jchipmunk
Copy link
Contributor

Kafdrop uses a heavy topic info object in all cases, even if additional data is not needed on UI side. It is proposed to add the ability to manage enrichment of the topic info object using the following modes:

  • PartitionSize - specifies to enrich topic info with partition size (first offset, last offset and size)
  • TopicConfig - specifies to enrich topic info with topic configuration

These changes are based on the following assumptions:

  • cluster-overview and broker-detail views do not need data about size of partitions
  • message-inspector and consumer-detail views do not need data about topic configuration

Kafdrop uses a heavy topic info object in all cases, even if additional data is not needed on UI side. It is proposed to add the ability to manage enrichment of the topic info object using the following modes:
* PartitionSize - specifies to enrich topic info with partition size (first offset, last offset and size)
* TopicConfig - specifies to enrich topic info with topic configuration

These changes are based on the following assumptions:
* cluster-overview and broker-detail views do not need data about size of partitions
* message-inspector and consumer-detail views do not need data about topic configuration
@xakassi
Copy link

xakassi commented Jun 5, 2020

Please, merge this. It greatly improved the speed of page loading!

@ekoutanov
Copy link
Member

I like the idea of only pulling what we need from Kafka; however, I have some reservations regarding the implementation.

The code in the PR is trying to reuse the same data models, and is performing "enrichment" on them, which makes it hard to follow, and also results in a bunch of fields that are uninitialised. I would much rather us split up the value objects and use "lighter" VOs where appropriate, without any enricher methods.

@jonas-grgt
Copy link

jonas-grgt commented Nov 12, 2020

I must agree with @ekoutanov. Also adding a kind of flag argument to alter the behaviour of a method isn't a nice solution. I'm refering the clean code here.
Next to that I was looking at the code and noticed there was actually no need for the deviation since the configMap of the TopicVo is never used. Or am I overlooking something here ? If not I believe removing the code that fills up the unused configMap would speed up the page loading also and would be less invasive.

@jchipmunk jchipmunk closed this Jul 8, 2022
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.

4 participants