-
Notifications
You must be signed in to change notification settings - Fork 13
perf: Change HDD read params default values #615
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
This change targets tuning the chunkserver read scheme default behavior into providing better performance in a general basis. Signed-off-by: Dave <[email protected]>
Summary of ChangesHello @dmga44, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces performance tuning adjustments to the chunkserver's HDD read scheme. By modifying two key default parameters, the aim is to optimize read operations for better general performance. The changes involve increasing the amount of data processed per read job while simultaneously reducing the degree of parallelism for read jobs per client connection, reflecting a strategic shift in how disk reads are managed to improve overall efficiency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request updates the default values for HDD read parameters to improve performance. Specifically, MAX_BLOCKS_PER_HDD_READ_JOB is increased from 8 to 16, and MAX_PARALLEL_HDD_READ_JOBS_PER_CS_ENTRY is decreased from 16 to 1. The changes are applied consistently across the codebase, including documentation and configuration files.
My main concern, detailed in a specific comment, is the potential for performance regression in certain workloads due to the reduction in parallelism for read jobs. While this may benefit sequential reads on HDDs, it could negatively impact other scenarios. More details on the performance analysis would be beneficial.
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
Performance tuning changes to optimize HDD read parameters in the chunkserver configuration by adjusting default values for better general performance.
- Increased maximum blocks per HDD read job from 8 to 16 to enable larger single read operations
- Decreased maximum parallel HDD read jobs per client connection from 16 to 1 to reduce contention
- Updated configuration documentation to reflect the new default values
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/data/sfschunkserver.cfg.in | Updated configuration template with new default values and comments |
| src/chunkserver/network_worker_thread.h | Modified constant definitions for HDD read job parameters |
| src/admin/dump_config_command.cc | Updated default configuration map with new parameter values |
| doc/sfschunkserver.cfg.5.adoc | Updated documentation to reflect new default values |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This change targets tuning the chunkserver read scheme default behavior into providing better performance in a general basis. MAX_BLOCKS_PER_HDD_READ_JOB = 8 -> 16 MAX_PARALLEL_HDD_READ_JOBS_PER_CS_ENTRY = 16 -> 1 Signed-off-by: Dave <[email protected]>
This change targets tuning the chunkserver read scheme default behavior
into providing better performance in a general basis:
MAX_BLOCKS_PER_HDD_READ_JOB = 8 -> 16
MAX_PARALLEL_HDD_READ_JOBS_PER_CS_ENTRY = 16 -> 1
Signed-off-by: Dave [email protected]