Skip to content

fix: update map rather than create a new map #3302

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.

Sign up for GitHub

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

Merged
hindessm merged 4 commits into from
Sep 23, 2025
Merged

fix: update map rather than create a new map #3302

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c5c3ad
chore: allocate correct size maps when known
hindessm Sep 21, 2025
a071473
chore: use complex literal when all fields are known
hindessm Sep 22, 2025
e39a29c
chore: minor refactoring after modernize
hindessm Sep 22, 2025
735af1d
bug: clear map rather than create new map
hindessm Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (ca *clusterAdmin) CreateTopic(topic string, detail *TopicDetail, validateO
return errors.New("you must specify topic details")
}

topicDetails := make(map[string]*TopicDetail)
topicDetails := make(map[string]*TopicDetail, 1)
topicDetails[topic] = detail
Comment thread
hindessm marked this conversation as resolved.
Outdated

request := NewCreateTopicsRequest(
Expand Down Expand Up @@ -366,7 +366,7 @@ func (ca *clusterAdmin) ListTopics() (map[string]TopicDetail, error) {
return nil, err
}

topicsDetailsMap := make(map[string]TopicDetail)
topicsDetailsMap := make(map[string]TopicDetail, len(metadataResp.Topics))

var describeConfigsResources []*ConfigResource

Expand All @@ -375,7 +375,7 @@ func (ca *clusterAdmin) ListTopics() (map[string]TopicDetail, error) {
NumPartitions: int32(len(topic.Partitions)),
}
if len(topic.Partitions) > 0 {
topicDetails.ReplicaAssignment = map[int32][]int32{}
topicDetails.ReplicaAssignment = make(map[int32][]int32, len(topic.Partitions))
for _, partition := range topic.Partitions {
topicDetails.ReplicaAssignment[partition.ID] = partition.Replicas
}
Expand Down Expand Up @@ -479,7 +479,7 @@ func (ca *clusterAdmin) CreatePartitions(topic string, count int32, assignment [
return ErrInvalidTopic
}

topicPartitions := make(map[string]*TopicPartition)
topicPartitions := make(map[string]*TopicPartition, 1)
topicPartitions[topic] = &TopicPartition{Count: count, Assignment: assignment}
Comment thread
hindessm marked this conversation as resolved.
Outdated

request := &CreatePartitionsRequest{
Expand Down Expand Up @@ -615,8 +615,8 @@ func (ca *clusterAdmin) DeleteRecords(topic string, partitionOffsets map[int32]i
partitionPerBroker[broker] = append(partitionPerBroker[broker], partition)
}
for broker, partitions := range partitionPerBroker {
topics := make(map[string]*DeleteRecordsRequestTopic)
recordsToDelete := make(map[int32]int64)
topics := make(map[string]*DeleteRecordsRequestTopic, 1)

@puellanivis puellanivis Sep 22, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this down to line 623, and convert it and lines 623–625 to a complex literal.

@hindessm hindessm Sep 22, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

recordsToDelete := make(map[int32]int64, len(partitions))
for _, p := range partitions {
recordsToDelete[p] = partitionOffsets[p]
}
Expand Down Expand Up @@ -1032,7 +1032,7 @@ func (ca *clusterAdmin) ListConsumerGroups() (allGroups map[string]string, err e
return
}

groups := make(map[string]string)
groups := make(map[string]string, len(response.Groups))
maps.Copy(groups, response.Groups)

@puellanivis puellanivis Sep 22, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
groups := make(map[string]string, len(response.Groups))
maps.Copy(groups, response.Groups)
groups := maps.Clone(response.Groups)

@hindessm hindessm Sep 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than just do this one, I decided to make an additional commit with all of the uncontentious changes from your earlier comments on #3297.

One of the changes, which looks logically correct to me, causes a linter error so I may live to regret making the wider changes.


groupMaps <- groups
Expand Down Expand Up @@ -1204,7 +1204,7 @@ func (ca *clusterAdmin) DescribeLogDirs(brokerIds []int32) (allLogDirs map[int32
close(logDirsResults)
close(errChan)

allLogDirs = make(map[int32][]DescribeLogDirsResponseDirMetadata)
allLogDirs = make(map[int32][]DescribeLogDirsResponseDirMetadata, len(brokerIds))
for logDirsResult := range logDirsResults {
allLogDirs[logDirsResult.id] = logDirsResult.logdirs
}
Expand Down
Loading