-
Notifications
You must be signed in to change notification settings - Fork 44
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_size
in 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
errors
import 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_size
values to verify the fallback logic works as expected.
chanSizeInt, err := strconv.Atoi(chanSize)
vermeer/apps/master/bl/scheduler_bl.go:46
- The code uses
logrus
for 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)
auth_token_factor=1234 |
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. |
Copilot uses AI. Check for mistakes.
@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 - TODO
Doc - Done
Doc - No Need