Conversation
Summary of ChangesHello @lgsilva3087, 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 significantly enhances the testing framework's ability to simulate and test environments with multiple active Metadata Server (MDS) instances, particularly for Key-Value (KV) backends. By refactoring configuration generation and introducing specific logic for multi-MDS setups, the changes streamline the process of setting up complex test scenarios, ensuring more robust testing of distributed file system metadata operations. Highlights
Changelog
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extends the testing framework to support multiple active MDS instances for KV backends. The changes involve refactoring the shell script for test setup to generate different configurations for multiple MDSs. The logic seems correct and aligns with the goal, and includes a good bug fix. I've found one area where the code can be improved for better maintainability by reducing duplication.
There was a problem hiding this comment.
Pull request overview
This pull request extends the testing framework to support multiple active MDS (Metadata Server) instances for KV backends like FoundationDB. The changes refactor configuration generation to enable multiple MDS instances to coexist with different port assignments while sharing common configuration.
Changes:
- Refactored configuration generation by extracting common MDS configuration into a reusable function
- Added support for multiple MDS instances with FDB backend, where MDS 0 uses global ports and additional MDSs use their own local ports
- Fixed a bug where the wrong port variable was being stored for the matots port
lgsilva3087
force-pushed
the
tests-allow-multiple-mds
branch
from
dddaa1b to
2ba4823
Compare
lgsilva3087
requested review from
GigaCronos,
antuan96314,
dmga44,
ralcolea,
rolysr and
uristdwarf
lgsilva3087
force-pushed
the
tests-allow-multiple-mds
branch
from
2ba4823 to
027fb20
Compare
lgsilva3087
force-pushed
the
tests-allow-multiple-mds
branch
from
027fb20 to
49ebc65
Compare
The matocs port was used twice (for matocs and for matots) in the function used to generate the master and shadows config files for testing. Even if the TS module is deprecated, it should point to the correct port. Signed-off-by: guillex <guillex@leil.io>
Refactor metadata server configuration to support multiple MDS
instances in test environment:
- Extract common MDS configuration into create_mds_common_cfg()
function to reduce code duplication
- Add create_sfsmds_cfg_() function for FDB-based MDS setup
- Support differentiated port allocation: MDS 0 uses global
ports (like Master), additional MDSs use their own local ports
(similar to Shadow servers)
- Update add_metadata_server_() to use new MDS configuration
when FDB backend is enabled
- Standardize variable references using ${var} syntax in the modified
sections
Signed-off-by: guillex <guillex@leil.io>
lgsilva3087
force-pushed
the
tests-allow-multiple-mds
branch
from
49ebc65 to
0294021
Compare
Extends the testing framework to support multiple active MDS instances in automated tests.
The change is intended for KV backends.
Related to: LS-340