feat: parallelize azure fetch of vms #673
Conversation
…nto azure-parallel-fetch
WalkthroughThis pull request updates the project’s dependencies and enhances resource fetching concurrency. In the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant vmProvider
participant Pool
participant Worker
Client->>vmProvider: GetResource(ctx)
vmProvider->>Pool: Submit tasks for each resource group
alt Concurrent Processing
Pool->>Worker: Execute resource fetch
Worker-->>Pool: Return VM and Network data or error
end
Pool-->>vmProvider: Aggregate results and errors
vmProvider->>Client: Return combined resources/errors
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/providers/azure/vm.go (2)
43-50: Initialize a fixed-size ants pool.Creating a pool with
ants.NewPool(10)andWithPreAlloc(true)is a valid approach. If Azure throttling arises, consider adjusting the pool size.
136-139: Return only the first error from an array of errors.You currently report just the first error among all gathered. Consider aggregating all errors or returning a multierror if you want better visibility into all failures:
if len(errs) > 0 { - return list, fmt.Errorf("encountered %d errors during resource fetching: %v", len(errs), errs[0]) + combined := multierror.Append(nil, errs...) + return list, fmt.Errorf("encountered %d errors during resource fetching: %v", len(errs), combined.ErrorOrNil()) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(2 hunks)pkg/providers/azure/vm.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test Builds (1.22.x, windows-latest)
- GitHub Check: Lint Test
- GitHub Check: release-test
🔇 Additional comments (8)
go.mod (2)
220-220: Upgrade golang.org/x/sync with caution.Upgrading to
v0.11.0may include API or performance changes. Please ensure downstream code remains compatible and run tests to confirm.
188-188:Details
❓ Verification inconclusive
Confirm the stability and compatibility of the new ants dependency.
You've added
github.com/panjf2000/ants/v2 v2.11.2for concurrency. This looks appropriate; however, ensure no license or compatibility conflicts arise within the project.Please run this script to confirm there are no known security issues or higher stable versions:
🏁 Script executed:
#!/bin/bash # Description: Check the latest release of ants and any CVEs/security advisories. # Pull latest ants tags from GitHub git ls-remote --tags https://github.com/panjf2000/ants.git # Optionally, check for CVEs related to ants library # (Replace <TOKEN> with a valid GitHub token) gh api graphql -F orgName='panjf2000' -f query=' query($orgName: String!){ securityVulnerabilities(first:10, ecosystem: GO, package: "ants") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } } ' -H "Authorization: bearer <TOKEN>"Length of output: 3255
Review Update – Verify Ants Dependency Stability and Security
Thegit ls-remotecommand confirms that versionv2.11.2exists and appears to be the latest stable release. However, the CVE/security check did not complete successfully due to invalid GitHub credentials. Please re-run the security advisory lookup using a valid token (or verify manually) to ensure there are no known vulnerabilities or license conflicts affecting the project.
- Confirm that
v2.11.2is indeed the intended stable version.- Re-run the GraphQL API check with valid credentials to complete the CVE verification.
pkg/providers/azure/vm.go (6)
5-6: New imports for concurrency and formatting.Adding
"fmt"and"sync"for printing and synchronization is appropriate for the upcoming parallel execution and error aggregation.
13-13: Introducing ants for goroutine pooling.The
antslibrary is well-suited for capping and managing parallel tasks. Just keep an eye on potential Azure API rate limits when choosing the pool size.
38-41: Synchronized error handling.Using separate
errMuand sliceerrsfor concurrency-safely appending errors is correct.
51-63: Task submission for each resource group.The loop submission approach is correct, and copying the
groupvariable prevents closure capture issues. Good practice.
64-126: Concurrent VM and NIC fetching logic.Collecting errors and guarding writes to
listwith a mutex is a proper pattern. This ensures you don’t lose partial results on transient errors.
129-133: Error from task submission handled gracefully.Locking around the global
errsslice uponpool.Submitfailure ensures thread safety. Good job.
Summary by CodeRabbit
New Features
Chores