feat: add new hash expire cmd to pika #2883
Conversation
1. pkhget pkhset 2. pkhexpire pkhexpireat 3. pkhexpiretime pkhpersist 4. pkhttl
WalkthroughThe changes introduce a new "PKHash" data type and associated commands in the access control and storage systems. Key modifications include the addition of enumerators, command constants, and methods for managing PKHashes, enhancing the functionality of both the command processing framework and the storage backend. Formatting improvements and organizational adjustments are also made across various files for clarity and readability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandHandler
participant Storage
User->>CommandHandler: PKHSet(key, field, value)
CommandHandler->>Storage: Store key, field, value
Storage-->>CommandHandler: Confirmation
CommandHandler-->>User: Success
sequenceDiagram
participant User
participant CommandHandler
participant Storage
User->>CommandHandler: PKHGet(key, field)
CommandHandler->>Storage: Retrieve key, field
Storage-->>CommandHandler: Value, metadata
CommandHandler-->>User: Value or error
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 Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (14)
src/storage/src/redis_pkhashes.cc (7)
23-63: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
66-142: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
144-205: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
207-274: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
276-333: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
335-394: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
397-452: Consider adding logging for better debugging and monitoring.The function is correctly implemented with proper error handling and validation checks. Adding logging can help in better debugging and monitoring.
Consider adding logging statements at key points in the function.
src/storage/src/storage.cc (6)
480-484: Review Error Handling inGetDBInstanceThe
PKHExpirefunction includes error handling for invalid TTL values, which is appropriate. However, theGetDBInstancefunction lacks explicit error handling, which might be a concern if the slot index retrieval or instance access could fail. Consider adding error handling toGetDBInstanceto ensure robustness.
GetDBInstance: No explicit error handling observed.PKHExpire: Handles invalid TTL values correctly.Analysis chain
LGTM! Verify error handling.
The function is correctly implemented. Ensure that
GetDBInstanceandPKHExpirehandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in `GetDBInstance` and `PKHExpire`. # Test: Search for the function implementations. Expect: Proper error handling. rg --type cpp -A 5 $'std::unique_ptr<Redis>& Storage::GetDBInstance' rg --type cpp -A 5 $'Status Redis::PKHExpire'Length of output: 2096
486-490: Potential Issue: Lack of Error Handling inGetDBInstanceThe
GetDBInstancefunction does not have explicit error handling, which could lead to issues if an invalid instance is returned. Consider adding error handling to ensure robustness. ThePKHExpireatfunction inRediscorrectly handles invalid timestamps.
- Review the
GetDBInstancefunction for potential error handling improvements.Analysis chain
LGTM! Verify error handling.
The function is correctly implemented. Ensure that
GetDBInstanceandPKHExpireathandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in `GetDBInstance` and `PKHExpireat`. # Test: Search for the function implementations. Expect: Proper error handling. rg --type cpp -A 5 $'std::unique_ptr<Redis>& Storage::GetDBInstance' rg --type cpp -A 5 $'Status Redis::PKHExpireat'Length of output: 1159
Script:
#!/bin/bash # Description: Check for error handling related to `GetDBInstance`. # Search for any error handling or checks related to `GetDBInstance`. rg --type cpp 'GetDBInstance' -A 3 | grep -i 'if\|try\|catch\|error\|exception'Length of output: 1873
504-508: Improve Error Handling inGetDBInstanceThe
GetDBInstancefunction lacks explicit error handling, which could lead to issues if an invalid index is accessed. Consider adding error checks to handle such cases. ThePKHTTLfunction inRedishas partial error handling for specific conditions, which is a positive aspect.
- File:
src/storage/src/storage.cc- Function:
GetDBInstanceAnalysis chain
LGTM! Verify error handling.
The function is correctly implemented. Ensure that
GetDBInstanceandPKHTTLhandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in `GetDBInstance` and `PKHTTL`. # Test: Search for the function implementations. Expect: Proper error handling. rg --type cpp -A 5 $'std::unique_ptr<Redis>& Storage::GetDBInstance' rg --type cpp -A 5 $'Status Redis::PKHTTL'Length of output: 1084
Script:
#!/bin/bash # Description: Gather more information on error handling in `GetDBInstance` and `PKHTTL`. # Check for error handling in `GetDBInstance`. rg --type cpp -A 10 'std::unique_ptr<Redis>& Storage::GetDBInstance' # Check for error handling in `PKHTTL`. rg --type cpp -A 20 'Status Redis::PKHTTL'Length of output: 2518
510-513: Ensure Robust Error Handling inGetDBInstanceThe
PKHGetfunction inRedishas appropriate error handling for type and existence checks. However,GetDBInstancelacks explicit error handling, which could lead to issues if the instance retrieval fails. Consider adding error handling to ensure robustness.
GetDBInstanceinsrc/storage/src/storage.cclacks error handling.PKHGetinsrc/storage/src/redis_pkhashes.cchas adequate error handling.Analysis chain
LGTM! Verify error handling.
The function is correctly implemented. Ensure that
GetDBInstanceandPKHGethandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in `GetDBInstance` and `PKHGet`. # Test: Search for the function implementations. Expect: Proper error handling. rg --type cpp -A 5 $'std::unique_ptr<Redis>& Storage::GetDBInstance' rg --type cpp -A 5 $'Status Redis::PKHGet'Length of output: 1049
Script:
#!/bin/bash # Description: Gather more context around `GetDBInstance` and `PKHGet` for error handling. # Search for the full implementation of `GetDBInstance` to check for error handling. rg --type cpp -A 20 'std::unique_ptr<Redis>& Storage::GetDBInstance' # Search for the full implementation of `PKHGet` to check for error handling. rg --type cpp -A 20 'Status Redis::PKHGet'Length of output: 3083
498-502: Improve Error Handling inGetDBInstanceThe
GetDBInstancefunction lacks explicit error handling, which could lead to issues if an invalid index is accessed. Consider adding checks to ensure the index is valid before accessing the instance. ThePKHPersistfunction has some error handling mechanisms in place, which is good practice.
- File:
src/storage/src/storage.cc- Function:
GetDBInstanceAnalysis chain
LGTM! Verify error handling.
The function is correctly implemented. Ensure that
GetDBInstanceandPKHPersisthandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in `GetDBInstance` and `PKHPersist`. # Test: Search for the function implementations. Expect: Proper error handling. rg --type cpp -A 5 $'std::unique_ptr<Redis>& Storage::GetDBInstance' rg --type cpp -A 5 $'Status Redis::PKHPersist'Length of output: 1096
Script:
#!/bin/bash # Description: Retrieve complete implementations to verify error handling in `GetDBInstance` and `PKHPersist`. # Search for the complete implementation of `GetDBInstance` to check for error handling. rg --type cpp -A 20 $'std::unique_ptr<Redis>& Storage::GetDBInstance' # Search for the complete implementation of `PKHPersist` to check for error handling. rg --type cpp -A 20 $'Status Redis::PKHPersist'Length of output: 3101
515-518: Add Error Handling toGetDBInstanceand VerifyRedis::PKHSet
GetDBInstanceshould include error handling to manage invalid instance indices.- Ensure comprehensive error handling in
Redis::PKHSetto cover all possible error conditions.Please review and address these points to improve the robustness of the code.
Analysis chain
LGTM! Verify error handling.
The function is correctly implemented. Ensure that
GetDBInstanceandPKHSethandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in `GetDBInstance` and `PKHSet`. # Test: Search for the function implementations. Expect: Proper error handling. rg --type cpp -A 5 $'std::unique_ptr<Redis>& Storage::GetDBInstance' rg --type cpp -A 5 $'Status Redis::PKHSet'Length of output: 1054
Script:
#!/bin/bash # Description: Gather more lines to verify error handling in `GetDBInstance` and `Redis::PKHSet`. # Get additional context for `GetDBInstance` rg --type cpp -A 10 $'std::unique_ptr<Redis>& Storage::GetDBInstance(const std::string& key)' # Get additional context for `Redis::PKHSet` rg --type cpp -A 10 $'Status Redis::PKHSet(const Slice& key, const Slice& field, const Slice& value, int32_t* res)'Length of output: 211
Script:
#!/bin/bash # Description: Refine search to verify error handling in `GetDBInstance` and `Redis::PKHSet`. # Search for `GetDBInstance` with more context rg --type cpp -A 15 'std::unique_ptr<Redis>& Storage::GetDBInstance' # Search for `Redis::PKHSet` with more context rg --type cpp -A 15 'Status Redis::PKHSet'Length of output: 2388
src/pika_command.cc (1)
19-19: Add include guard forpika_pkhash.h.Ensure that the header file
pika_pkhash.his included only once to prevent potential redefinition errors.Consider adding an include guard in
pika_pkhash.hif it doesn't already exist.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- include/acl.h (1 hunks)
- include/pika_command.h (6 hunks)
- include/pika_pkhash.h (1 hunks)
- src/pika_client_conn.cc (1 hunks)
- src/pika_command.cc (10 hunks)
- src/pika_pkhash.cc (1 hunks)
- src/storage/include/storage/storage.h (18 hunks)
- src/storage/include/storage/storage_define.h (2 hunks)
- src/storage/src/base_filter.h (6 hunks)
- src/storage/src/base_value_format.h (5 hunks)
- src/storage/src/pkhash_data_value_format.h (1 hunks)
- src/storage/src/redis.cc (12 hunks)
- src/storage/src/redis.h (17 hunks)
- src/storage/src/redis_hashes.cc (37 hunks)
- src/storage/src/redis_pkhashes.cc (1 hunks)
- src/storage/src/storage.cc (36 hunks)
Files skipped from review due to trivial changes (2)
- src/pika_client_conn.cc
- src/storage/src/redis_hashes.cc
Additional comments not posted (147)
src/storage/include/storage/storage_define.h (2)
47-47: LGTM!The addition of the new enumerator
kPKHashDataCFis correct and consistent with the existing enumerators.The code changes are approved.
124-132: LGTM!The formatting changes improve readability without altering the logic of the function.
The code changes are approved.
src/storage/src/base_value_format.h (3)
21-31: LGTM!The addition of the new enumerators
kPKHashesandkNonesis correct and consistent with the existing enumerators.The code changes are approved.
34-35: LGTM!The update to the
DataTypeStringsarray ensures that all enumerators have corresponding string values.The code changes are approved.
52-53: LGTM!The formatting changes improve readability without altering the logic of the code.
The code changes are approved.
Also applies to: 100-101, 153-153, 163-167
include/pika_pkhash.h (7)
14-38: LGTM!The class
PKHExpireCmdis correctly implemented.The code changes are approved.
40-64: LGTM!The class
PKHExpireatCmdis correctly implemented.The code changes are approved.
65-89: LGTM!The class
PKHExpiretimeCmdis correctly implemented.The code changes are approved.
91-115: LGTM!The class
PKHPersistCmdis correctly implemented.The code changes are approved.
117-141: LGTM!The class
PKHTTLCmdis correctly implemented.The code changes are approved.
143-164: LGTM!The class
PKHGetCmdis correctly implemented.The code changes are approved.
166-187: LGTM!The class
PKHSetCmdis correctly implemented.The code changes are approved.
src/pika_pkhash.cc (14)
16-38: LGTM!The method
PKHExpireCmd::DoInitialis correctly implemented.The code changes are approved.
40-53: LGTM!The method
PKHExpireCmd::Dois correctly implemented.The code changes are approved.
55-80: LGTM!The method
PKHExpireatCmd::DoInitialis correctly implemented.The code changes are approved.
81-94: LGTM!The method
PKHExpireatCmd::Dois correctly implemented.The code changes are approved.
96-116: LGTM!The method
PKHExpiretimeCmd::DoInitialis correctly implemented.The code changes are approved.
117-130: LGTM!The method
PKHExpiretimeCmd::Dois correctly implemented.The code changes are approved.
132-150: LGTM!The method
PKHPersistCmd::DoInitialis correctly implemented.The code changes are approved.
151-164: LGTM!The method
PKHPersistCmd::Dois correctly implemented.The code changes are approved.
166-184: LGTM!The method
PKHTTLCmd::DoInitialis correctly implemented.The code changes are approved.
185-198: LGTM!The method
PKHTTLCmd::Dois correctly implemented.The code changes are approved.
200-207: LGTM!The method
PKHGetCmd::DoInitialis correctly implemented.The code changes are approved.
209-222: LGTM!The method
PKHGetCmd::Dois correctly implemented.The code changes are approved.
244-252: LGTM!The method
PKHSetCmd::DoInitialis correctly implemented.The code changes are approved.
254-265: LGTM!The method
PKHSetCmd::Dois correctly implemented.The code changes are approved.
src/storage/src/base_filter.h (5)
Line range hint
21-74: LGTM!The class
BaseMetaFilteris correctly implemented.The code changes are approved.
Line range hint
114-228: LGTM!The class
BaseDataFilteris correctly implemented. The constructor reformatting improves readability.The code changes are approved.
250-250: LGTM!The type alias
PKHashesMetaFilteris correctly implemented.The code changes are approved.
251-251: LGTM!The type alias
PKHashesMetaFilterFactoryis correctly implemented.The code changes are approved.
252-252: LGTM!The type alias
PKHashesDataFilteris correctly implemented.The code changes are approved.
include/acl.h (1)
55-55: LGTM!The addition of the
PKHASHenumerator to theAclCategoryenum is consistent with the PR objectives and summary.The code changes are approved.
include/pika_command.h (14)
141-141: LGTM!The addition of the
kCmdNamePKHSetconstant is consistent with the PR objectives and summary.The code changes are approved.
142-142: LGTM!The addition of the
kCmdNamePKHSetexconstant is consistent with the PR objectives and summary.The code changes are approved.
143-143: LGTM!The addition of the
kCmdNamePKHExpireconstant is consistent with the PR objectives and summary.The code changes are approved.
144-144: LGTM!The addition of the
kCmdNamePKHExpireatconstant is consistent with the PR objectives and summary.The code changes are approved.
145-145: LGTM!The addition of the
kCmdNamePKHExpiretimeconstant is consistent with the PR objectives and summary.The code changes are approved.
146-146: LGTM!The addition of the
kCmdNamePKHTTLconstant is consistent with the PR objectives and summary.The code changes are approved.
147-147: LGTM!The addition of the
kCmdNamePKHPersistconstant is consistent with the PR objectives and summary.The code changes are approved.
148-148: LGTM!The addition of the
kCmdNamePKHGetconstant is consistent with the PR objectives and summary.The code changes are approved.
149-149: LGTM!The addition of the
kCmdNamePKHExistsconstant is consistent with the PR objectives and summary.The code changes are approved.
150-150: LGTM!The addition of the
kCmdNamePKHDelconstant is consistent with the PR objectives and summary.The code changes are approved.
151-151: LGTM!The addition of the
kCmdNamePKHLenconstant is consistent with the PR objectives and summary.The code changes are approved.
152-152: LGTM!The addition of the
kCmdNamePKHStrlenconstant is consistent with the PR objectives and summary.The code changes are approved.
153-153: LGTM!The addition of the
kCmdNamePKHIncrbyconstant is consistent with the PR objectives and summary.The code changes are approved.
154-154: LGTM!The addition of the
kCmdNamePKHIncrbyfloatconstant is consistent with the PR objectives and summary.The code changes are approved.
src/storage/src/redis.cc (3)
Line range hint
30-42: LGTM!The constructor is correctly initializing the new column family options for
pika_hash_data_cf.The code changes are approved.
102-111: LGTM!The function is correctly setting up the new column family options for
pika_hash_data_cf.The code changes are approved.
218-218: LGTM!The function is correctly including
kPKHashDataCFin the list of column families to compact.The code changes are approved.
src/storage/src/redis.h (8)
246-248: LGTM!The function is correctly implemented to retrieve column family handles for PK Hashes.
The code changes are approved.
253-254: LGTM!The function is correctly implemented to set expiration time for PK Hash fields.
The code changes are approved.
255-256: LGTM!The function is correctly implemented to set expiration time for PK Hash fields based on a timestamp.
The code changes are approved.
257-258: LGTM!The function is correctly implemented to retrieve expiration times for PK Hash fields.
The code changes are approved.
259-260: LGTM!The function is correctly implemented to retrieve TTL for PK Hash fields.
The code changes are approved.
261-262: LGTM!The function is correctly implemented to make PK Hash fields persistent by removing their expiration.
The code changes are approved.
263-263: LGTM!The function is correctly implemented to retrieve the value of a PK Hash field.
The code changes are approved.
264-264: LGTM!The function is correctly implemented to set the value of a PK Hash field.
The code changes are approved.
src/storage/include/storage/storage.h (16)
26-26: LGTM!The reordering of include statements is acceptable.
The code changes are approved.
28-28: LGTM!The inclusion of the new header file is necessary for the new functionality.
The code changes are approved.
Line range hint
98-104: LGTM!The addition of the
operator+method forKeyInfostruct is correct and useful for combining key information.The code changes are approved.
111-113: LGTM!The addition of the
ttlfield toValueStatusstruct is necessary for managing time-to-live information.The code changes are approved.
157-159: LGTM!The addition of the
Operationenum is useful for defining various operations in the background task.The code changes are approved.
171-173: LGTM!The addition of the constructor and destructor for the
Storageclass is necessary for proper initialization and cleanup.The code changes are approved.
256-257: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
262-263: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
272-273: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
482-483: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
506-507: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
516-517: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
590-591: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
753-754: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
1000-1001: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
1035-1036: LGTM!The method signature update ensures consistent formatting and clarity.
The code changes are approved.
src/storage/src/storage.cc (1)
492-496: LGTM! Verify error handling.The function is correctly implemented. Ensure that
GetDBInstanceandPKHExpiretimehandle errors correctly.The code changes are approved.
Run the following script to verify error handling:
src/pika_command.cc (73)
486-487: Correct initialization ofPKHSetCmd.The command
PKHSetCmdis correctly initialized and inserted into the command table.The code changes are approved.
490-492: Correct initialization ofPKHExpireCmd.The command
PKHExpireCmdis correctly initialized and inserted into the command table.The code changes are approved.
494-496: Correct initialization ofPKHExpireatCmd.The command
PKHExpireatCmdis correctly initialized and inserted into the command table.The code changes are approved.
498-500: Correct initialization ofPKHExpiretimeCmd.The command
PKHExpiretimeCmdis correctly initialized and inserted into the command table.The code changes are approved.
502-503: Correct initialization ofPKHTTLCmd.The command
PKHTTLCmdis correctly initialized and inserted into the command table.The code changes are approved.
506-508: Correct initialization ofPKHPersistCmd.The command
PKHPersistCmdis correctly initialized and inserted into the command table.The code changes are approved.
510-511: Correct initialization ofPKHGetCmd.The command
PKHGetCmdis correctly initialized and inserted into the command table.The code changes are approved.
56-57: Consistent formatting forCompactCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
60-61: Consistent formatting forCompactRangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
80-81: Consistent formatting forFlushallCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
85-86: Consistent formatting forFlushdbCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
159-160: Consistent formatting forClearCacheCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
162-163: Consistent formatting forLastsaveCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
247-248: Consistent formatting forSetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
251-253: Consistent formatting forGetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
256-258: Consistent formatting forDelCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
264-265: Consistent formatting forIncrCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
269-269: Consistent formatting forIncrbyCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
273-274: Consistent formatting forIncrbyfloatCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
277-278: Consistent formatting forDecrCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
282-282: Consistent formatting forDecrbyCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
286-286: Consistent formatting forGetsetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
290-290: Consistent formatting forAppendCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
293-295: Consistent formatting forMgetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
303-303: Consistent formatting forSetnxCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
306-307: Consistent formatting forSetexCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
310-311: Consistent formatting forPsetexCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
315-315: Consistent formatting forDelvxCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
318-319: Consistent formatting forMsetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
322-323: Consistent formatting forMsetnxCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
327-328: Consistent formatting forGetrangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
332-332: Consistent formatting forSetrangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
335-337: Consistent formatting forStrlenCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
340-342: Consistent formatting forExistsCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
346-347: Consistent formatting forExpireCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
351-352: Consistent formatting forPexpireCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
355-357: Consistent formatting forExpireatCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
360-362: Consistent formatting forPexpireatCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
365-366: Consistent formatting forTtlCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
369-370: Consistent formatting forPttlCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
373-375: Consistent formatting forPersistCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
378-379: Consistent formatting forTypeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
391-391: Consistent formatting forPKSetexAtCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
394-395: Consistent formatting forPKScanRangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
398-399: Consistent formatting forPKRScanRangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
404-405: Consistent formatting forHDelCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
408-409: Consistent formatting forHSetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
412-414: Consistent formatting forHGetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
417-419: Consistent formatting forHGetallCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
422-424: Consistent formatting forHExistsCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
427-428: Consistent formatting forHIncrbyCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
431-433: Consistent formatting forHIncrbyfloatCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
436-438: Consistent formatting forHKeysCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
441-443: Consistent formatting forHLenCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
446-448: Consistent formatting forHMgetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
451-452: Consistent formatting forHMsetCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
455-456: Consistent formatting forHSetnxCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
459-461: Consistent formatting forHStrlenCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
464-466: Consistent formatting forHValsCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
469-470: Consistent formatting forHScanCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
473-474: Consistent formatting forHScanxCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
477-478: Consistent formatting forPKHScanRangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
481-482: Consistent formatting forPKHRScanRangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
514-516: Consistent formatting forLIndexCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
518-519: Consistent formatting forLInsertCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
522-524: Consistent formatting forLLenCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
526-527: Consistent formatting forBLPopCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
530-531: Consistent formatting forLPopCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
535-535: Consistent formatting forLPushCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
538-539: Consistent formatting forLPushxCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
543-544: Consistent formatting forLRangeCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
546-547: Consistent formatting forLRemCmd.The formatting change improves readability by aligning parameters across multiple lines.
The code changes are approved.
549-550: **Consistent formatting for
| class PKHashDataValue : public InternalValue { | ||
| public: | ||
| /* | ||
| * The header of the Value field is initially initialized to knulltype | ||
| */ | ||
| explicit PKHashDataValue(const rocksdb::Slice& user_value) : InternalValue(DataType::kNones, user_value) {} | ||
| virtual ~PKHashDataValue() {} | ||
|
|
||
| virtual rocksdb::Slice Encode() { | ||
| size_t usize = user_value_.size(); | ||
| size_t needed = usize + kSuffixReserveLength + kTimestampLength * 2; | ||
| char* dst = ReAllocIfNeeded(needed); | ||
| char* start_pos = dst; | ||
|
|
||
| memcpy(dst, user_value_.data(), user_value_.size()); | ||
| dst += user_value_.size(); | ||
| memcpy(dst, reserve_, kSuffixReserveLength); | ||
| dst += kSuffixReserveLength; | ||
| EncodeFixed64(dst, ctime_); | ||
| dst += kTimestampLength; | ||
| EncodeFixed64(dst, etime_); | ||
| dst += kTimestampLength; // todo(DDD) 待确认，看这个是否需要。 | ||
|
|
||
| return rocksdb::Slice(start_pos, needed); | ||
| } | ||
|
|
||
| private: | ||
| const size_t kDefaultValueSuffixLength = kSuffixReserveLength + kTimestampLength * 2; | ||
| }; |
There was a problem hiding this comment.
LGTM! But address the TODO comment.
The PKHashDataValue class is correctly implemented. However, the TODO comment in the Encode method needs to be addressed.
The code changes are approved.
Do you want me to help address the TODO comment or open a GitHub issue to track this task?
| class ParsedPKHashDataValue : public ParsedInternalValue { | ||
| public: | ||
| // Use this constructor after rocksdb::DB::Get(), since we use this in | ||
| // the implement of user interfaces and may need to modify the | ||
| // original value suffix, so the value_ must point to the string | ||
| explicit ParsedPKHashDataValue(std::string* value) : ParsedInternalValue(value) { | ||
| if (value_->size() >= kPKHashDataValueSuffixLength) { | ||
| user_value_ = rocksdb::Slice(value_->data(), value_->size() - kPKHashDataValueSuffixLength); | ||
| memcpy(reserve_, value_->data() + user_value_.size(), kSuffixReserveLength); | ||
| ctime_ = DecodeFixed64(value_->data() + user_value_.size() + kSuffixReserveLength); | ||
| etime_ = DecodeFixed64(value_->data() + user_value_.size() + kSuffixReserveLength + kTimestampLength); | ||
| } | ||
| } | ||
|
|
||
| // Use this constructor in rocksdb::CompactionFilter::Filter(), | ||
| // since we use this in Compaction process, all we need to do is parsing | ||
| // the rocksdb::Slice, so don't need to modify the original value, value_ can be | ||
| // set to nullptr | ||
| explicit ParsedPKHashDataValue(const rocksdb::Slice& value) : ParsedInternalValue(value) { | ||
| if (value.size() >= kPKHashDataValueSuffixLength) { | ||
| user_value_ = rocksdb::Slice(value.data(), value.size() - kPKHashDataValueSuffixLength); | ||
| memcpy(reserve_, value.data() + user_value_.size(), kSuffixReserveLength); | ||
| ctime_ = DecodeFixed64(value.data() + user_value_.size() + kSuffixReserveLength); | ||
| etime_ = DecodeFixed64(value_->data() + user_value_.size() + kSuffixReserveLength + kTimestampLength); | ||
| } | ||
| } | ||
|
|
||
| virtual ~ParsedPKHashDataValue() = default; | ||
|
|
||
| void SetEtimeToValue() override { | ||
| if (value_) { | ||
| char* dst = const_cast<char*>(value_->data()) + value_->size() - kTimestampLength; | ||
| EncodeFixed64(dst, etime_); | ||
| } | ||
| } | ||
|
|
||
| void SetCtimeToValue() override { | ||
| if (value_) { | ||
| char* dst = const_cast<char*>(value_->data()) + value_->size() - kTimestampLength - kTimestampLength; | ||
| EncodeFixed64(dst, ctime_); | ||
| } | ||
| } | ||
|
|
||
| void SetReserveToValue() { | ||
| if (value_) { | ||
| char* dst = const_cast<char*>(value_->data()) + value_->size() - kPKHashDataValueSuffixLength; | ||
| memcpy(dst, reserve_, kSuffixReserveLength); | ||
| } | ||
| } | ||
|
|
||
| virtual void StripSuffix() override { | ||
| if (value_) { | ||
| value_->erase(value_->size() - kPKHashDataValueSuffixLength, kPKHashDataValueSuffixLength); | ||
| } | ||
| } | ||
|
|
||
| void SetTimestamp(int64_t timestamp) { | ||
| etime_ = timestamp; | ||
| SetEtimeToValue(); | ||
| } | ||
|
|
||
| protected: | ||
| virtual void SetVersionToValue() override {}; | ||
|
|
||
| private: | ||
| const size_t kPKHashDataValueSuffixLength = kSuffixReserveLength + kTimestampLength * 2; | ||
| }; |
There was a problem hiding this comment.
LGTM! But address the empty SetVersionToValue method.
The ParsedPKHashDataValue class is correctly implemented. However, the SetVersionToValue method is empty and might need to be implemented.
The code changes are approved.
Do you want me to help implement the SetVersionToValue method or open a GitHub issue to track this task?
Summary by CodeRabbit
New Features
PKHASHto enhance access control functionality.PKHSet,PKHGet, and expiration commands.Bug Fixes
Documentation