-
Notifications
You must be signed in to change notification settings - Fork 1
feat: process compass sink concurrently #81
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
Conversation
There was a problem hiding this comment.
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 introduces concurrent processing in the Compass sink to handle record sinking concurrently.
- Added sync.WaitGroup and error channel for concurrent error handling
- Wraps record processing in a goroutine for parallel execution
2ecec21 to
da32851
Compare
There was a problem hiding this comment.
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 introduces concurrent processing for sinking records to the compass service. Key changes include:
- Employing golang.org/x/sync/errgroup to process records concurrently.
- Removing error wrapping in favor of returning raw errors.
- Updating the test’s expected error message accordingly.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plugins/sinks/compass/sink_test.go | Updated expected error message to match changes in error propagation. |
| plugins/sinks/compass/sink.go | Introduced concurrent processing using errgroup and modified error returns. |
ae4d388 to
c67c063
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Add concurrent processing to the Compass sink using errgroup, update tests for the new error flow, and ensure CI artifacts are uniquely named per job.
- Introduce an
errgroup.Groupto runsendcalls in parallel inSink. - Simplify error propagation by returning underlying errors directly.
- Parameterize coverage artifact names in the GitHub Actions workflow with
${{ strategy.job-index }}.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plugins/sinks/compass/sink.go | Use errgroup for concurrent sinking and simplify error returns |
| plugins/sinks/compass/sink_test.go | Adjust expected error message to match the simplified error path |
| .github/workflows/test.yml | Add job-index suffix to coverage artifact names |
Comments suppressed due to low confidence (1)
plugins/sinks/compass/sink.go:99
- [nitpick] The variable name
errChis ambiguous; consider renaming togrouporegto clarify that it’s anerrgroup.Group.
errCh := errgroup.Group{}
c67c063 to
f594bcf
Compare
f594bcf to
ecf1a1f
Compare
Add concurrent processing during compass sink