-
Notifications
You must be signed in to change notification settings - Fork 45
refactor(vermeer): make startChan's size configurable #328
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 makes the startChan buffer size configurable via a new start_chan_size option and adds error handling to fall back to a default if parsing fails.
- Introduce
start_chan_sizein the master configuration - Read and convert the config value at startup, with fallback and logging on errors
- Update channel initialization to use the configured size
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| vermeer/config/master.ini | Add new start_chan_size configuration option |
| vermeer/apps/master/bl/scheduler_bl.go | Load, parse, and handle errors for start_chan_size before creating startChan |
Comments suppressed due to low confidence (3)
vermeer/apps/master/bl/scheduler_bl.go:21
- Remove the unused
errorsimport to clean up the import block.
"errors"
vermeer/apps/master/bl/scheduler_bl.go:44
- Consider adding unit tests to cover valid and invalid
start_chan_sizevalues to verify the fallback logic works as expected.
chanSizeInt, err := strconv.Atoi(chanSize)
vermeer/apps/master/bl/scheduler_bl.go:46
- The code uses
logrusfor logging but it isn't imported; add the appropriate import (e.g.,github.com/sirupsen/logrus).
logrus.Errorf("failed to convert start_chan_size to int: %v", err)
| task_parallel_num=1 | ||
| auth=none | ||
| auth_token_factor=1234 | ||
| auth_token_factor=1234 |
Copilot
AI
May 22, 2025
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.
Add a descriptive comment for start_chan_size in master.ini to explain its purpose and valid values.
| auth_token_factor=1234 | |
| auth_token_factor=1234 | |
| ; The initial size of the channel buffer used for communication between components. | |
| ; Valid values: Any positive integer. A larger value may improve performance for high-throughput systems. |
|
@imbajin This commit can be approved |
Purpose of the PR
Use configurable options to configure the size of start_chan add error handling at the same time.
Solve problem below:
The default size of startChan is 10 and cannot be changed. The queue size may need to be manually configured by users to reduce congestion.Main Changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need