-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: keyspace error for stream type #2705
fix: keyspace error for stream type #2705
Conversation
WalkthroughThe recent changes enhance the handling of keyspace information in the Pika database. Key modifications include resizing vectors based on Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin
participant PikaAdmin as PikaAdmin
participant PikaDB as PikaDB
participant Storage as Storage
participant Redis as Redis
Admin->>PikaAdmin: Request Keyspace Info
PikaAdmin->>PikaDB: Run Key Scan
PikaDB->>Storage: Get Key Num
Storage->>Redis: Scan for Key Types
Redis->>Storage: Return Key Types Count
Storage->>PikaDB: Return Key Infos
PikaDB->>PikaAdmin: Return Key Infos
PikaAdmin->>Admin: Display Keyspace Info including Streams
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 (
|
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/pika_admin.cc (2 hunks)
- src/pika_db.cc (1 hunks)
- src/storage/src/redis.cc (1 hunks)
- src/storage/src/storage.cc (1 hunks)
Additional comments not posted (4)
src/storage/src/redis.cc (1)
390-390
: Ensure correct handling of stream keys inScanKeyNum
.This change correctly adds the handling of stream keys, aligning with the PR's objective to fix the keyspace error for stream types.
src/pika_db.cc (1)
98-102
: OptimizeRunKeyScan
by directly assigning results.This change optimizes the
RunKeyScan
method by directly assigning the result ofGetKeyNum
tonew_key_infos
, which eliminates unnecessary intermediate iteration and should improve performance.src/storage/src/storage.cc (1)
1826-1826
: Ensure dynamic resizing ofkey_infos
aligns with data type changes.This change dynamically resizes the
key_infos
vector to accommodate all data types, which aligns with the PR's objective to correctly count stream types in the keyspace. Good use ofDataType::kNones
to determine the size, ensuring that all types are considered.src/pika_admin.cc (1)
1230-1231
: Addition of stream keys toInfoKeyspace
output.The addition of stream keys to the
InfoKeyspace
output is consistent with the PR's objective to handle stream types in the keyspace command. This change correctly appends stream key information to the output, enhancing the command's functionality.
@@ -1201,7 +1201,7 @@ void InfoCmd::InfoKeyspace(std::string& info) { | |||
key_scan_info = db_item.second->GetKeyScanInfo(); | |||
key_infos = key_scan_info.key_infos; | |||
duration = key_scan_info.duration; | |||
if (key_infos.size() != 5) { | |||
if (key_infos.size() != (size_t)(storage::DataType::kNones)) { |
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.
Ensure correct handling of key_infos
size.
The check for key_infos.size()
against storage::DataType::kNones
is crucial for ensuring that the array has the expected number of elements. However, this check should be moved to a more appropriate place where key_infos
is populated or modified, to ensure that it always has the correct size before being accessed.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pika_admin.cc (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/pika_admin.cc
please fix the ci failure |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- include/pika_db.h (1 hunks)
Additional comments not posted (2)
include/pika_db.h (2)
27-27
: The addition ofstreams
tokey_infos
aligns with the PR's objectives to handle stream types correctly.
31-31
: Ensure that all functions usingkey_infos
are updated to handle the newstreams
type correctly.Verification successful
The extracted code snippets confirm that the
streams
type is correctly handled in bothsrc/pika_admin.cc
andsrc/storage/src/redis.cc
. Specifically:
- In
src/pika_admin.cc
, thestreams
type is referenced and processed in thetmp_stream
output.- In
src/storage/src/redis.cc
, thestreams
type is handled in theScanStreamsKeyNum
function.This indicates that the functions using
key_infos
have been updated to handle the newstreams
type correctly.
src/pika_admin.cc
:
- Lines involving
key_infos[5]
forStreams_keys
.src/storage/src/redis.cc
:
- The
ScanStreamsKeyNum
function is called forkey_infos[5]
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all functions using `key_infos` handle the new `streams` type correctly. # Test: Search for functions using `key_infos`. Expect: Functions should handle the new `streams` type. rg --type cpp $'key_infos'Length of output: 3451
Script:
#!/bin/bash # Extract relevant code snippets from src/pika_admin.cc to verify handling of the streams type rg --type cpp --context 5 'key_infos' src/pika_admin.cc # Extract relevant code snippets from src/storage/src/redis.cc to verify handling of the streams type rg --type cpp --context 5 'key_infos' src/storage/src/redis.ccLength of output: 3657
* fix keyspace error for stream type * fix dbsize bug * fix keyscaninfo init error --------- Co-authored-by: wangshaoyi <wangshaoyi@360.cn>
Fixed the problem that the keyspace command does not count the number of stream types.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes