Skip to content

Concurrency#18

Merged
kv1sidisi merged 3 commits into
mainfrom
concurrency
Jul 16, 2025
Merged

Concurrency#18
kv1sidisi merged 3 commits into
mainfrom
concurrency

Conversation

@kv1sidisi

Copy link
Copy Markdown
Owner

No description provided.

@kv1sidisi kv1sidisi requested a review from Copilot July 16, 2025 18:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds concurrency support to the metrics collection process in the collector. The changes enable parallel processing of metrics while respecting configurable concurrency limits.

  • Introduces concurrent goroutine execution for metric collection using sync.WaitGroup
  • Implements a semaphore pattern to limit maximum concurrent operations
  • Maintains sequential collection logic but executes it in parallel for better performance

Comment on lines +82 to +95
go func(mc config.Metric) {
defer wg.Done()

smph <- struct{}{}
defer func() {
<-smph
}()

if len(mc.SubMetrics) == 0 {
c.collectSimpleMetric(ch, mc)
} else {
c.collectComplicatedMetric(ch, mc)
}
}(metricConfig)

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

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

The semaphore acquisition could block indefinitely if the context is cancelled or the operation needs to be interrupted. Consider using a select statement with context cancellation to make this interruptible.

Copilot uses AI. Check for mistakes.
Comment on lines +90 to 100
if len(mc.SubMetrics) == 0 {
c.collectSimpleMetric(ch, mc)
} else {
c.collectComplicatedMetric(ch, mc)
}
}(metricConfig)
}

wg.Wait()

c.logger.Info("Metrics collection finished")

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

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

The WaitGroup.Wait() call could block indefinitely if any goroutine panics or hangs. Consider adding a timeout or context cancellation to prevent the collector from hanging indefinitely.

Copilot uses AI. Check for mistakes.
wg.Add(1)

go func(mc config.Metric) {
defer wg.Done()

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

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

Goroutines that panic will not be recovered and could cause issues. Consider adding panic recovery with defer recover() to ensure the WaitGroup is properly decremented and semaphore is released even if collectSimpleMetric or collectComplicatedMetric panic.

Suggested change
defer wg.Done()
defer func() {
if r := recover(); r != nil {
c.logger.Error("Panic recovered in goroutine", "metric", mc.Name, "error", r)
}
wg.Done()
}()

Copilot uses AI. Check for mistakes.
@kv1sidisi kv1sidisi merged commit 156005a into main Jul 16, 2025
2 checks passed
@kv1sidisi kv1sidisi deleted the concurrency branch July 16, 2025 18:39
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