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

Feature/node page partition #1033

Merged

Conversation

jgrammen-agilitypr
Copy link
Contributor

#983 - add the total partitions and partitions percentage column

@tchiotludo
Copy link
Owner

Hi thanks for that, really appreciate.

I've tested quickly and it don't seems to works, on a multi node cluster, all node have the total count of partitions.

I've started an implementation but don't have more time to dig it (that produce the same results but on more modern way:

    @Get("api/{cluster}/node/partitions")
    @Operation(tags = {"topic"}, summary = "partition counts")
    public List<NodePartition> nodePartitions(String cluster) throws ExecutionException, InterruptedException {
        List<String> topicNames = this.topicRepository.all(cluster, TopicRepository.TopicListView.ALL, Optional.empty());
        List<Topic> topics = this.topicRepository.findByName(cluster, topicNames);


        long totalPartitions = topics
            .stream()
            .mapToInt(t -> t.getPartitions().size())
            .count();

        return topics
            .stream()
            .flatMap(topic -> topic.getPartitions().stream())
            .flatMap(partition -> partition.getNodes()
                .stream()
                .map(n -> NodePartition.builder()
                    .id(n.getId())
                    .countLeader(n.isLeader() ? 1 : 0)
                    .countInSyncReplicas(n.isInSyncReplicas() ? 1 : 0)
                    .build()
                )
            )
            .collect(Collectors.groupingBy(NodePartition::getId))
            .entrySet()
            .stream()
            .map(n -> NodePartition.builder()
                .id(n.getKey())
                .countLeader(n.getValue().stream().mapToInt(NodePartition::getCountLeader).sum())
                .countLeader(n.getValue().stream().mapToInt(NodePartition::getCountInSyncReplicas).sum())
                .build()
            )
            .collect(Collectors.toList());
    }

    @Introspected
    @Builder(toBuilder = true)
    @Value
    public static class NodePartition {
        int id;
        int countLeader;
        int countInSyncReplicas;
    }
  • Look at the part to fetch topics, my 2 lines will be faster (since we fetch all only 1 time, you are calling a lot of time this.topicRepository.findByName, that is slow on big cluster)
  • Since all topics is fetch, you can use stream api to count
  • By the same way, the stream api should work to count the number of partition per node
  • Always use a true model for return type on Controller, Map are undocumented by swagger
  • The better is also to add a unit test for java controller

The logic is wrong also on my side but used this compose to have a multiple node cluster.

On the ui side, the browser freeze waiting the 2 requests is done, please can you use the async request that we use on the topic List page (displaying node and fetch in async all the counter please) ?

…tup docker composer for optional commented second kafka node
@jgrammen-agilitypr
Copy link
Contributor Author

I have updated the pull request based on your feedback.
The Java code has been updated to match the more modern style (based on your example code, with a few minor tweaks)
The javascript was updated to match the new java code
setup a commented second kafka node in the composer file

@tchiotludo tchiotludo merged commit b78b35e into tchiotludo:dev Mar 17, 2022
@tchiotludo
Copy link
Owner

I've added a minor change on last commit in order to load truly async the partition repartion.

Thanks for the contributions !

@jgrammen-agilitypr
Copy link
Contributor Author

any chance a release with this feature is going to be made anytime soon?

@tchiotludo
Copy link
Owner

You can use the dev image to have early access

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