fix: parse min-blob-size error when min-blob-size ends with unit #2767
Conversation
WalkthroughThe recent updates include several key changes. In Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
wangshao1
force-pushed
the
fix/parse_conf_error
branch
from
257b18a
to
5652eb5
Compare
wangshao1
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/pika_conf.cc (1 hunks)
- src/storage/src/redis_zsets.cc (1 hunks)
- tests/integration/zset_test.go (1 hunks)
Additional comments not posted (2)
src/pika_conf.cc (1)
650-650: Updated method for parsing 'min-blob-size' configuration.The change from
GetConfInt64toGetConfInt64Humanallows for human-readable input formats (e.g., "10MB", "1GB"). This is a beneficial change as it enhances the flexibility and user-friendliness of configuration settings.src/storage/src/redis_zsets.cc (1)
1133-1133: Refinement in loop condition ofZRevrankThe modification in the loop condition from
left >= 0toleft > 0appears to be a deliberate change to avoid an off-by-one error. However, it's crucial to ensure that this change aligns with the intended functionality of theZRevrankmethod. This adjustment could potentially affect how ranks are calculated, especially for edge cases.Please verify through additional unit tests that this change does not inadvertently exclude any elements that should be included in the rank calculation, and that it behaves correctly across all possible inputs.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pika_conf.cc (1 hunks)
Additional comments not posted (1)
src/pika_conf.cc (1)
665-665: Confirm the change in configuration parsing formin-blob-size.The change from
GetConfInt64toGetConfInt64Humanformin-blob-sizesuggests an enhancement in how values are parsed, likely supporting more human-readable formats (e.g., "1M" instead of "1048576"). This is a positive change if documented properly and tested.Verification successful
Confirm the correct implementation of
GetConfInt64Human.The function
GetConfInt64Humanis used consistently across the codebase for parsing various configuration values. This suggests a standardized approach to handling human-readable integer formats. To ensure its correctness, let's verify the implementation details ofGetConfInt64Humaninbase_conf.cc.
Verify the implementation of
GetConfInt64Humaninbase_conf.cc.The previous attempt to capture the implementation details of
GetConfInt64Humanwas unsuccessful. We will now usergto locate the function definition inbase_conf.ccto ensure it correctly handles human-readable formats.
Extract the full implementation of
GetConfInt64Humanfrombase_conf.cc.We have located the definition of
GetConfInt64Human. Now, let's extract its full implementation to verify that it correctly handles human-readable formats.
Confirm the change in configuration parsing for
min-blob-size.The function
GetConfInt64Humancorrectly handles human-readable formats, converting 'K', 'M', and 'G' suffixes to their corresponding values in bytes. The change fromGetConfInt64toGetConfInt64Humanformin-blob-sizeand other configurations is properly implemented and enhances the usability of configuration settings.
src/pika_conf.cc: Line 665Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new parsing function handles various human-readable formats correctly. # Test: Search for the function `GetConfInt64Human` usage. Expect: Only occurances of the new signature. rg --type cpp $'GetConfInt64Human'Length of output: 1143
Script:
#!/bin/bash # Description: Verify the implementation of GetConfInt64Human function. # Search for the implementation of GetConfInt64Human in base_conf.cc ast-grep --lang cpp --pattern $'bool BaseConf::GetConfInt64Human(const std::string& name, int64_t* value) const { $$$ }'Length of output: 123
Script:
#!/bin/bash # Description: Verify the implementation of GetConfInt64Human function. # Search for the implementation of GetConfInt64Human in base_conf.cc rg 'bool BaseConf::GetConfInt64Human' --type cppLength of output: 158
Script:
#!/bin/bash # Description: Extract the full implementation of GetConfInt64Human function. # Extract the full implementation of GetConfInt64Human from base_conf.cc rg -A 20 'bool BaseConf::GetConfInt64Human' src/pstd/src/base_conf.ccLength of output: 676
Co-authored-by: wangshaoyi <wangshaoyi@360.cn>
Co-authored-by: wangshaoyi <wangshaoyi@360.cn>
Co-authored-by: wangshaoyi <wangshaoyi@360.cn>
Co-authored-by: wangshaoyi <wangshaoyi@360.cn>
Summary by CodeRabbit
Bug Fixes
New Features
ZREVRANKfunctionality in the Redis client for better reliability and coverage.