fix(topic): performance for read and creation #1393
Conversation
ivan-savciuc
added
the
no changelog
ivan-savciuc
force-pushed
the
ivans-fix-kafka-creation
branch
from
2aa4078 to
456dac3
Compare
|
@ivan-savciuc hey is this for v4 or v3, or both? |
| // No error means topic exists | ||
| if err == nil { | ||
| client := m.(*aiven.Client) | ||
| if isTopicExists(ctx, client, project, serviceName, topicName) { |
There was a problem hiding this comment.
consider adding a comment above this block to briefly explain its purpose for clarity
| @@ -201,3 +201,11 @@ func DeleteTopicFromCache(projectName, serviceName, topicName string) { | |||
| } | |||
| t.Unlock() | |||
| } | |||
|
|
|||
| // GetFullQueue retrieves a topics queue | |||
| func (t *kafkaTopicCache) GetFullQueue(projectName, serviceName string) []string { | |||
There was a problem hiding this comment.
ensure the method name accurately represents its purpose; If the function retrieves a specific type of queue or has a unique behavior, be more explicit in the name
| t.RLock() | ||
| defer t.RUnlock() | ||
|
|
||
| return t.inQueue[projectName+serviceName] |
There was a problem hiding this comment.
consider handling a scenario where the key might not exist in the inQueue map; directly accessing a non-existent key will return a nil slice, which might be unexpected for the function caller
| "golang.org/x/exp/slices" | ||
| ) | ||
|
|
||
| var onceForService sync.Map |
There was a problem hiding this comment.
consider renaming the variable to be more descriptive, like onceCheckTopicForService or something similar
|
|
||
| var err error | ||
| once, _ := onceForService.LoadOrStore(project+"/"+serviceName, new(sync.Once)) | ||
| once.(*sync.Once).Do(func() { |
There was a problem hiding this comment.
add comments for better clarity, especially on the significance of using sync.Once
| c.SetV1List(project, serviceName, list) | ||
| }) | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
this approach can be confusing: if possible, consider handling the error within the once.Do block, or refactoring the approach to make the flow clearer
| return false | ||
| } | ||
|
|
||
| if slices.Contains(c.GetV1List(project, serviceName), topic) || slices.Contains(c.GetFullQueue(project, serviceName), topic) { |
There was a problem hiding this comment.
for better performance, if the topic is found in the first c.GetV1List check, there's no need to check in the second c.GetFullQueue call: you can return true immediately
Serpentiel
changed the title
|
Converting this patch for v4 instead of v5 (current master), will open another RP |
About this change—what it does
Fix Kafka Topic create and read performance