fix: change storage ttl time from seconds to milliseconds #2857
Conversation
…undation#2822) * change storage ttl time from seconds to milliseconds --------- Co-authored-by: liuyuecai <[email protected]> test space
WalkthroughThe recent changes significantly enhance clarity and precision in time handling throughout the codebase, moving from seconds to milliseconds for time-to-live (TTL) parameters. This shift improves code readability, ensuring developers are aware of the expected time units. Additionally, various formatting adjustments have been made to enhance overall code style without altering functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Codebase
participant TimeUtility
Developer->>Codebase: Implement changes for time handling
Codebase->>TimeUtility: Fetch current time in milliseconds
TimeUtility-->>Codebase: Return current time
Codebase->>Codebase: Update method signatures with ttl_millsec
Codebase-->>Developer: Changes implemented successfully
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: 0
Outside diff range, codebase verification and nitpick comments (1)
.github/workflows/pika.yml (1)
67-67: Added echo statement for clarity.The
echo "hello"statement serves as a simple log message to indicate the start of the integration test setup. Consider adding a more descriptive message.- echo "hello" + echo "Starting integration test setup..."
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (38)
- .github/workflows/pika.yml (3 hunks)
- include/pika_cache.h (1 hunks)
- include/pika_kv.h (14 hunks)
- src/net/examples/performance/server.cc (1 hunks)
- src/pika_admin.cc (2 hunks)
- src/pika_bit.cc (1 hunks)
- src/pika_cache.cc (1 hunks)
- src/pika_cache_load_thread.cc (1 hunks)
- src/pika_conf.cc (2 hunks)
- src/pika_geo.cc (1 hunks)
- src/pika_kv.cc (40 hunks)
- src/pika_list.cc (1 hunks)
- src/pika_server.cc (5 hunks)
- src/pika_zset.cc (5 hunks)
- src/pstd/include/env.h (2 hunks)
- src/pstd/src/env.cc (1 hunks)
- src/storage/include/storage/storage.h (15 hunks)
- src/storage/src/base_data_value_format.h (4 hunks)
- src/storage/src/base_filter.h (2 hunks)
- src/storage/src/base_meta_value_format.h (5 hunks)
- src/storage/src/base_value_format.h (4 hunks)
- src/storage/src/lists_filter.h (1 hunks)
- src/storage/src/lists_meta_value_format.h (5 hunks)
- src/storage/src/redis.h (6 hunks)
- src/storage/src/redis_hashes.cc (16 hunks)
- src/storage/src/redis_lists.cc (9 hunks)
- src/storage/src/redis_sets.cc (15 hunks)
- src/storage/src/redis_streams.cc (1 hunks)
- src/storage/src/redis_strings.cc (26 hunks)
- src/storage/src/redis_zsets.cc (17 hunks)
- src/storage/src/storage.cc (16 hunks)
- src/storage/src/strings_filter.h (1 hunks)
- src/storage/src/strings_value_format.h (4 hunks)
- src/storage/src/zsets_filter.h (1 hunks)
- src/storage/tests/keys_test.cc (1 hunks)
- src/storage/tests/lists_filter_test.cc (2 hunks)
- src/storage/tests/strings_filter_test.cc (1 hunks)
- src/storage/tests/strings_test.cc (9 hunks)
Files skipped from review due to trivial changes (9)
- src/pika_admin.cc
- src/pika_bit.cc
- src/pika_cache.cc
- src/pika_conf.cc
- src/pika_geo.cc
- src/pika_list.cc
- src/pika_server.cc
- src/pika_zset.cc
- src/storage/src/redis_streams.cc
Additional context used
Learnings (1)
src/storage/src/redis_sets.cc (1)
Learnt from: wangshao1 PR: OpenAtomFoundation/pika#2744 File: src/storage/src/redis_sets.cc:0-0 Timestamp: 2024-06-20T09:25:58.037Z Learning: Refactor involving extensive code changes should be deferred to subsequent versions to avoid risks before major releases. This is tracked under issue #2751 for the Pika project.
Additional comments not posted (179)
src/storage/tests/strings_filter_test.cc (1)
24-24: LGTM! Verify the function usage in the test cases.The change from
SetRelativeTimestamptoSetRelativeTimeByMillsecaligns with the transition to milliseconds. Ensure that all test cases using this function are updated accordingly.Verification successful
Function Usage Verified in Test Cases
The
SetRelativeTimeByMillsecfunction is correctly used in the test cases, includingstrings_filter_test.ccandlists_filter_test.cc. The transition to milliseconds has been properly integrated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all test cases using `SetRelativeTimeByMillsec`. # Test: Search for the function usage in test files. Expect: Only occurrences of the new function name. rg --type cc -A 5 $'SetRelativeTimeByMillsec'Length of output: 71
Script:
#!/bin/bash # Description: Verify all test cases using `SetRelativeTimeByMillsec`. # Test: Search for the function usage in test files without specifying the file type. rg -A 5 'SetRelativeTimeByMillsec'Length of output: 3759
src/storage/src/strings_filter.h (1)
23-23: LGTM! Verify the function usage in the codebase.The change to use
pstd::NowMillis()for time retrieval aligns with the transition to milliseconds. Ensure that all usages of theFiltermethod are consistent with this change.src/net/examples/performance/server.cc (1)
89-89: LGTM! Verify the variable type usage in the codebase.The change to use
pstd::TimeTypeforstandedenhances type safety and clarity. Ensure that all usages of these variables are consistent with the new type.src/storage/src/base_data_value_format.h (3)
43-44: Encoding logic forctime_is sound.The bitwise operation correctly sets the highest bit for positive
ctime_values. Ensure consistency with the decoding logic.
62-63: Decoding logic forctime_is consistent.The bitwise operation correctly clears the highest bit, ensuring accurate restoration of
ctime_.Also applies to: 75-76
87-88:SetCtimeToValuemethod logic is consistent.The encoding logic for
ctime_is consistent with other parts of the code, ensuring correctness.src/storage/src/base_value_format.h (3)
44-44: Initialization ofctime_with milliseconds is appropriate.The constructor now uses
pstd::NowMillis(), aligning with the shift to millisecond granularity.
54-56: MethodSetRelativeTimeByMillseccorrectly uses milliseconds.The method now aligns with the new time granularity, using
pstd::NowMillis()for calculations.
124-126: MethodSetRelativeTimestampcorrectly uses milliseconds.The method now accepts
ttl_millsecand usespstd::NowMillis(), aligning with the new time granularity.src/pstd/include/env.h (3)
20-21: Introduction ofTimeTypeimproves readability.The type alias
TimeTypeenhances consistency in handling time-related values.
66-66: Return type change forNowMicrosimproves consistency.The return type is now
TimeType, aligning with the new type alias for better consistency.
68-69: New functionNowMillisaligns with millisecond granularity.The function provides a consistent method for retrieving the current time in milliseconds, using
TimeType.src/storage/src/zsets_filter.h (1)
83-83: Change to milliseconds for time retrieval approved.The update to use
pstd::NowMillis()for obtaining the current time in milliseconds simplifies the code and aligns with the objective of enhancing time precision. Ensure that any logic relying on time comparisons is verified to behave correctly with this increased precision.Verification successful
Change to milliseconds for time retrieval verified.
The update to use
pstd::NowMillis()for obtaining the current time in milliseconds is consistent across the codebase. The logic involving time comparisons and expiration settings appears to be correctly aligned with the increased precision. Specifically, the following areas were checked:
src/storage/src/zsets_filter.h: Time comparisons withcur_meta_etime_.src/storage/src/strings_filter.h: Time comparisons and expiration logic.src/storage/src/base_meta_value_format.h: Version updates based on time.src/storage/src/base_value_format.h: Expiration time settings.These areas should behave correctly with the increased precision. If there are any specific scenarios or edge cases, further testing may be required to ensure correctness.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of changing time retrieval to milliseconds on time-dependent logic. # Test: Search for all time comparisons involving `unix_time`. rg --type cpp -A 5 $'unix_time'Length of output: 10547
src/storage/src/lists_filter.h (1)
91-91: Change to milliseconds for time retrieval approved.The modification to use
pstd::NowMillis()for time retrieval in milliseconds is consistent with the objective of improved precision. Verify that time-dependent logic continues to function as expected with this change.src/storage/src/strings_value_format.h (3)
143-153: Set timestamp encoding to milliseconds approved.The methods
SetCtimeToValueandSetEtimeToValuecorrectly encode timestamps with the significant bit for milliseconds. Ensure that these methods are used consistently and correctly throughout the codebase.Verification successful
Consistent usage of timestamp encoding methods confirmed.
The methods
SetCtimeToValueandSetEtimeToValueare consistently defined and used across multiple files in the codebase, ensuring that timestamps are encoded with the significant bit for milliseconds as intended. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `SetCtimeToValue` and `SetEtimeToValue` methods. # Test: Search for all occurrences of the methods to ensure consistent usage. rg --type cpp -A 5 $'SetCtimeToValue|SetEtimeToValue'Length of output: 5177
85-98: Timestamp decoding to milliseconds approved.The decoding logic correctly handles both milliseconds and seconds, ensuring backward compatibility. Verify that the conversions are accurate and consistent across the codebase.
Verification successful
Timestamp Decoding Logic Verified Across Codebase
The timestamp decoding logic that handles conversions from seconds to milliseconds is consistently and accurately applied across the codebase. This ensures backward compatibility and correctness of the timestamps.
- Instances found in:
src/storage/src/base_meta_value_format.hsrc/storage/src/strings_value_format.hsrc/storage/src/lists_meta_value_format.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of timestamp decoding logic. # Test: Search for all decoding operations involving `ctime_` and `etime_`. rg --type cpp -A 5 $'DecodeFixed64'Length of output: 22763
Script:
#!/bin/bash # Search for instances where `ctime_` and `etime_` are assigned values to verify timestamp conversion logic. rg --type cpp -A 5 'ctime_ = ' rg --type cpp -A 5 'etime_ = 'Length of output: 14021
38-44: Timestamp encoding to milliseconds approved.The changes to encode timestamps with a significant bit for distinguishing milliseconds from seconds are well-implemented. Ensure that the logic correctly handles both new and legacy timestamp formats.
Verification successful
To ensure the timestamp encoding logic is consistently applied across the codebase, let's verify other instances where timestamps are encoded using
EncodeFixed64. We need to check if they follow the same logic of setting the most significant bit for milliseconds. I'll search for instances wherectime_andetime_are used in conjunction withEncodeFixed64to confirm the correct logic is applied.
Timestamp encoding logic is consistently applied across the codebase.
The encoding logic for distinguishing between milliseconds and seconds using the most significant bit is correctly implemented in the relevant files. The instances of
ctime_andetime_being encoded withEncodeFixed64confirm that the logic is applied consistently.
- Files verified include:
src/storage/src/strings_value_format.hsrc/storage/src/lists_meta_value_format.hsrc/storage/src/base_data_value_format.hsrc/storage/src/base_meta_value_format.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of timestamp encoding logic. # Test: Search for all encoding operations involving `ctime_` and `etime_`. rg --type cpp -A 5 $'EncodeFixed64'Length of output: 28866
Script:
#!/bin/bash # Description: Verify if `ctime_` and `etime_` are consistently encoded with the correct logic. # Search for instances of `ctime_` and `etime_` in conjunction with `EncodeFixed64`. rg --type cpp -A 5 'ctime_' | grep 'EncodeFixed64' rg --type cpp -A 5 'etime_' | grep 'EncodeFixed64'Length of output: 1543
src/pika_cache_load_thread.cc (2)
122-122: Conversion of TTL from milliseconds to seconds.The conversion logic
ttl_millsec > 0 ? ttl_millsec / 1000 : ttl_millseccorrectly handles the conversion from milliseconds to seconds. Ensure this logic is consistently applied where necessary.
116-117: Ensure correct handling of TTL in milliseconds.The variable
ttl_millsecis used to store TTL in milliseconds. Ensure that all related logic and function calls handle this correctly.Verification successful
Correct handling of TTL in milliseconds confirmed.
The variable
ttl_millsecis consistently handled as milliseconds throughout the codebase. Conversions to seconds are performed where necessary, such as when interfacing with cache functions. No issues were found with the current implementation.
- Files reviewed:
src/pika_cache_load_thread.cc,src/storage/src/redis_sets.cc,src/storage/src/redis_zsets.cc,src/storage/src/redis_lists.cc,src/storage/src/redis_hashes.cc,src/storage/src/storage.cc,src/storage/src/redis_strings.cc,include/pika_kv.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all usages of `ttl_millsec` are correctly handling milliseconds. # Test: Search for `ttl_millsec` usage. Expect: Correct conversion and usage in related functions. rg --type cc $'ttl_millsec'Length of output: 53
Script:
#!/bin/bash # Description: Verify that all usages of `ttl_millsec` are correctly handling milliseconds. # Test: Search for `ttl_millsec` usage in C++ files. Expect: Correct conversion and usage in related functions. rg --type cpp 'ttl_millsec'Length of output: 20828
src/storage/src/base_meta_value_format.h (6)
42-48: Bitwise operation for timestamp differentiation.The use of the most significant bit to differentiate between seconds and milliseconds is a clever approach. Ensure that this bit manipulation is consistently applied throughout the codebase.
53-57: Update version using milliseconds.The
UpdateVersionmethod now uses milliseconds, aligning with the changes for timestamp precision. This is consistent with the overall objective of enhancing time handling.
78-91: Decoding logic for timestamps with bitwise checks.The decoding logic correctly checks the most significant bit to determine the original unit of the timestamp and converts it to milliseconds if necessary. Ensure this logic is applied consistently.
109-122: Decoding logic for timestamps with bitwise checks.This section mirrors the previous decoding logic, ensuring consistency across different constructors. The logic is sound and aligns with the objectives.
143-152: Encoding logic for timestamps with bitwise checks.The encoding logic correctly applies the bitwise operation to set the most significant bit for milliseconds. This ensures consistency with the decoding logic.
202-206: Update version using milliseconds.This section mirrors the earlier update to the
UpdateVersionmethod, ensuring consistency across the codebase..github/workflows/pika.yml (2)
57-57: Increased verbosity forctest.Adding the
--verboseflag toctestenhances the visibility of test results, aiding in debugging and monitoring.
181-181: Increased verbosity forctest.This mirrors the earlier change to add verbosity, ensuring consistent test output across different jobs.
src/storage/src/lists_meta_value_format.h (5)
Line range hint
58-63: LGTM!The
UpdateVersionmethod now usespstd::NowMillis(), aligning with the millisecond-based timestamp handling.
106-115: LGTM!The decoding logic correctly handles the conversion from seconds to milliseconds, ensuring backward compatibility.
174-175: LGTM!The
SetCtimeToValuemethod correctly sets the most significant bit for milliseconds.
182-183: LGTM!The
SetEtimeToValuemethod correctly sets the most significant bit for milliseconds.
47-53: LGTM! Verify backward compatibility.The encoding logic correctly uses the most significant bit to distinguish between milliseconds and seconds. Ensure that the decoding logic elsewhere in the codebase correctly handles both formats for backward compatibility.
Verification successful
Backward Compatibility Verified
The decoding logic correctly handles both milliseconds and seconds by checking the most significant bit. This ensures backward compatibility with previously stored data. The logic is consistent across relevant files:
src/storage/src/lists_meta_value_format.hsrc/storage/src/base_data_value_format.hsrc/storage/src/strings_value_format.hThese files contain logic to check the most significant bit and convert seconds to milliseconds if necessary.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that decoding logic handles both milliseconds and seconds. # Test: Search for decoding logic. Expect: Correct handling of the most significant bit. rg --type cpp -A 5 'DecodeFixed64'Length of output: 22763
src/storage/tests/lists_filter_test.cc (2)
122-122: LGTM!The method name change to
SetRelativeTimeByMillsecmaintains consistency across the test cases.
105-105: LGTM! Verify method implementation.The method name change to
SetRelativeTimeByMillsecimproves clarity by specifying milliseconds. Ensure the method implementation aligns with this change.Verification successful
Method Implementation Verified:
SetRelativeTimeByMillsecThe method
SetRelativeTimeByMillsecis implemented insrc/storage/src/base_value_format.hand is used in multiple files, confirming its integration and functionality. No issues found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `SetRelativeTimeByMillsec`. # Test: Search for the method implementation. Expect: Correct handling of milliseconds. ast-grep --lang cpp --pattern 'void SetRelativeTimeByMillsec(int milliseconds) { $$$ }'Length of output: 89
Script:
#!/bin/bash # Description: Search for any implementation of `SetRelativeTimeByMillsec` in the codebase. # Test: Search for any method named `SetRelativeTimeByMillsec`. ast-grep --lang cpp --pattern '$_ SetRelativeTimeByMillsec($$$) { $$$ }'Length of output: 74
Script:
#!/bin/bash # Description: Search for any function containing "SetRelativeTime" in its name. # Test: Search for any function with "SetRelativeTime" in its name. ast-grep --lang cpp --pattern '$_ SetRelativeTime$$$($$$) { $$$ }'Length of output: 68
Script:
#!/bin/bash # Description: List all functions in the file where `SetRelativeTimeByMillsec` is used to find related functions. # Test: List all function definitions in `src/storage/tests/lists_filter_test.cc`. ast-grep --lang cpp --pattern '$_ $_($$$) { $$$ }' src/storage/tests/lists_filter_test.ccLength of output: 91
Script:
#!/bin/bash # Description: Search for the function `SetRelativeTimeByMillsec` across the entire codebase. # Test: Search for any occurrence of `SetRelativeTimeByMillsec` in the codebase. rg 'SetRelativeTimeByMillsec'Length of output: 770
src/storage/src/base_filter.h (2)
31-31: LGTM!The use of
pstd::NowMillis()simplifies the code and aligns with the millisecond-based timestamp handling.
182-183: LGTM!The use of
pstd::NowMillis()simplifies the code and aligns with the millisecond-based timestamp handling.include/pika_cache.h (1)
70-70: Improved parameter clarity inExpireat.Renaming the parameter from
ttltottl_secenhances clarity by explicitly indicating the unit of time. Ensure that corresponding documentation and usage in the codebase are updated to reflect this change.src/pstd/src/env.cc (2)
225-228: Addition ofNowMillisfunction.The new
NowMillisfunction provides a useful utility for retrieving the current time in milliseconds. Ensure that its usage is consistent with the intended granularity.
220-220: Verify the impact of changingNowMicrosreturn type.Changing the return type of
NowMicrosfromuint64_ttoTimeTypemay affect its usage elsewhere in the codebase. Ensure that all dependent code is compatible withTimeType.include/pika_kv.h (14)
39-44: Improved variable naming inSetCmd.The renaming of
sec_tottl_millsecand its consistent reset inClearimproves clarity by explicitly indicating the unit of time.
72-72: Improved variable naming inGetCmd.The renaming of
sec_tottl_millsec_enhances clarity by specifying the unit of time.
118-118: Improved variable naming inIncrCmd.Changing
expired_timestamp_sec_toexpired_timestamp_millsec_clarifies the unit of time for expiration.
143-143: Improved variable naming inIncrbyCmd.The renaming to
expired_timestamp_millsec_makes the time unit explicit, enhancing readability.
168-168: Improved variable naming inIncrbyfloatCmd.Renaming to
expired_timestamp_millsec_clarifies the time unit, improving code readability.
263-263: Improved variable naming inAppendCmd.Renaming to
expired_timestamp_millsec_enhances clarity regarding the time unit.
354-354: Improved variable naming inSetexCmd.Changing
sec_tottl_sec_clarifies the time unit, improving code readability.
379-379: Improved variable naming inPsetexCmd.Renaming to
ttl_millsecmakes the time unit explicit, enhancing clarity.
543-543: Improved variable naming inStrlenCmd.Renaming to
ttl_millsecclarifies the time unit, improving code readability.
584-584: Improved variable naming inExpireCmd.Changing
sec_tottl_sec_makes the time unit explicit, enhancing clarity.
608-608: Improved variable naming inPexpireCmd.Renaming to
ttl_millsecclarifies the time unit, improving code readability.
632-632: Improved variable naming inExpireatCmd.Changing
time_stamp_totime_stamp_sec_clarifies the time unit, enhancing clarity.
655-655: Improved variable naming inPexpireatCmd.Renaming to
time_stamp_millsec_clarifies the time unit, improving code readability.
812-814: Improved variable naming inPKSetexAtCmd.Changing
time_stamp_totime_stamp_sec_and updatingClearenhances clarity by specifying the time unit.src/storage/src/redis.h (31)
120-120: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
121-121: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
122-122: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
123-123: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
124-124: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
133-133: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totimestamp_millsecto reflect the unit change to milliseconds.
134-134: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totimestamp_millsecto reflect the unit change to milliseconds.
135-135: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totimestamp_millsecto reflect the unit change to milliseconds.
136-136: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totimestamp_millsecto reflect the unit change to milliseconds.
137-137: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totimestamp_millsecto reflect the unit change to milliseconds.
145-145: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
146-146: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
147-147: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
148-148: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
149-149: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
152-152: Parameter name update to milliseconds.The parameter name
expired_timestamp_sechas been correctly updated toexpired_timestamp_millsecto reflect the unit change to milliseconds.
159-159: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
160-160: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
166-166: Parameter name update to milliseconds.The parameter name
expired_timestamp_sechas been correctly updated toexpired_timestamp_millsecto reflect the unit change to milliseconds.
172-172: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
174-174: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
175-175: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
176-176: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
184-184: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totime_stamp_millsec_to reflect the unit change to milliseconds.
188-188: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
189-189: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated totimestamp_millsecto reflect the unit change to milliseconds.
191-191: Parameter name update to milliseconds.The parameter name
timestamphas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
201-201: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
258-258: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
279-279: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.
294-294: Parameter name update to milliseconds.The parameter name
ttlhas been correctly updated tottl_millsecto reflect the unit change to milliseconds.src/storage/tests/strings_test.cc (14)
47-47: Expiration time update to milliseconds.The expiration time has been correctly updated from
1second to100milliseconds.
74-76: Introduceexpired_timestamp_millsecfor consistency.The variable
expired_timestamp_millsecis introduced to replacetype_ttl, aligning with the transition to milliseconds.
78-81: UpdateAppendcalls to useexpired_timestamp_millsec.The
Appendfunction calls now correctly useexpired_timestamp_millsecto test expiration handling in milliseconds.Also applies to: 83-86
95-115: Update expiration time to milliseconds and adjust assertions.The expiration time is updated to milliseconds, and assertions are adjusted to validate the correct behavior.
122-125: UpdateAppendcall to useexpired_timestamp_millsec.The
Appendfunction call now correctly usesexpired_timestamp_millsecto test expiration handling in milliseconds.
363-364: Introduceexpired_timestamp_millsecfor consistency.The variable
expired_timestamp_millsecis introduced to replacetype_ttl, aligning with the transition to milliseconds.
368-371: UpdateIncrbycalls to useexpired_timestamp_millsec.The
Incrbyfunction calls now correctly useexpired_timestamp_millsecto test expiration handling in milliseconds.Also applies to: 376-378, 383-385
390-411: Update expiration time to milliseconds and adjust assertions.The expiration time is updated to milliseconds, and assertions are adjusted to validate the correct behavior.
421-427: UpdateIncrbycall to useexpired_timestamp_millsec.The
Incrbyfunction call now correctly usesexpired_timestamp_millsecto test expiration handling in milliseconds.
453-454: Introduceexpired_timestamp_millsecfor consistency.The variable
expired_timestamp_millsecis introduced to replacetype_ttl, aligning with the transition to milliseconds.
459-462: UpdateIncrbyfloatcalls to useexpired_timestamp_millsec.The
Incrbyfloatfunction calls now correctly useexpired_timestamp_millsecto test expiration handling in milliseconds.Also applies to: 463-466, 471-473
478-498: Update expiration time to milliseconds and adjust assertions.The expiration time is updated to milliseconds, and assertions are adjusted to validate the correct behavior.
505-510: UpdateIncrbyfloatcall to useexpired_timestamp_millsec.The
Incrbyfloatfunction call now correctly usesexpired_timestamp_millsecto test expiration handling in milliseconds.
986-987: Update expiration time to milliseconds and adjust assertions.The expiration time is updated to milliseconds, and assertions are adjusted to validate the correct behavior.
Also applies to: 997-1000, 1010-1011, 1019-1022, 1030-1030, 1039-1039
src/pika_kv.cc (15)
Line range hint
25-55:
LGTM! Ensure command options are handled correctly.The transition from seconds to milliseconds for TTL is handled correctly in the
SetCmd::DoInitialmethod. The logic for parsing and setting TTL is consistent with the command's options.
70-79: LGTM!The changes to use
ttl_millsecinSetCmd::Doare consistent with the transition to milliseconds. The logic for handling different conditions is correct.
114-114: LGTM!The conversion from milliseconds to seconds for updating the cache is handled correctly in
SetCmd::DoUpdateCache.
161-161: LGTM!The changes to retrieve TTL in milliseconds in
GetCmd::Doare consistent with the new logic.
191-191: LGTM!The conversion from milliseconds to seconds for updating the cache in
GetCmd::DoUpdateCacheis handled correctly.
260-260: LGTM!The handling of TTL in milliseconds in
IncrCmd::Dois consistent with the new logic.
324-324: LGTM!The handling of TTL in milliseconds in
IncrbyCmd::Dois consistent with the new logic.
363-363: Verify timestamp conversion logic.The conversion from milliseconds to seconds for the Redis protocol in
IncrbyCmd::ToRedisProtocolis appropriate, but verify that it does not lead to unexpected behavior.
389-389: LGTM!The handling of TTL in milliseconds in
IncrbyfloatCmd::Dois consistent with the new logic.
432-432: Verify timestamp conversion logic.The conversion from milliseconds to seconds for the Redis protocol in
IncrbyfloatCmd::ToRedisProtocolis appropriate, but verify that it does not lead to unexpected behavior.
563-563: LGTM!The handling of TTL in milliseconds in
AppendCmd::Dois consistent with the new logic.
598-598: Verify timestamp conversion logic.The conversion from milliseconds to seconds for the Redis protocol in
AppendCmd::ToRedisProtocolis appropriate, but verify that it does not lead to unexpected behavior.
709-710: LGTM!The conversion from milliseconds to seconds for updating the cache in
MgetCmd::DoUpdateCacheis handled correctly.
136-138: Verify precision loss in TTL conversion.The conversion from milliseconds to seconds for the Redis protocol is correct, but there is a TODO comment indicating potential precision loss.
299-299: Verify timestamp conversion logic.The conversion from milliseconds to seconds for the Redis protocol in
IncrCmd::ToRedisProtocolis appropriate, but verify that it does not lead to unexpected behavior.src/storage/include/storage/storage.h (3)
108-109: LGTM: Renamingttltottl_millsecinValueStatus.The changes correctly update the
ValueStatusstruct to use milliseconds for TTL, aligning with the PR objectives. The equality operator is also updated to reflect this change.
193-193: Consistency in renamingttltottl_millsecacross methods.The renaming of
ttltottl_millsecacross various methods in theStorageclass is consistent and aligns with the shift to milliseconds for TTL values. This improves clarity and precision in time-related operations.Also applies to: 201-201, 230-230, 241-241, 258-258, 263-263, 288-288, 296-296, 306-306, 337-337, 470-470, 543-543, 706-706, 963-963, 1013-1013, 1035-1035, 1125-1125
1030-1035: LGTM: New methodPTTLfor TTL in milliseconds.The addition of the
PTTLmethod provides functionality to retrieve the remaining TTL in milliseconds, aligning with the transition to millisecond precision.src/storage/src/redis_lists.cc (4)
Line range hint
473-506: LGTM: Update toLRangeWithTTLfor TTL in milliseconds.The method now correctly calculates TTL in milliseconds using
pstd::NowMillis(), aligning with the transition to millisecond precision.
Line range hint
1098-1128: LGTM: Update toListsExpirefor TTL in milliseconds.The method now sets expiration times in milliseconds, aligning with the transition to millisecond precision.
Line range hint
1175-1204: LGTM: Update toListsExpireatfor timestamp in milliseconds.The method now sets expiration timestamps in milliseconds, aligning with the transition to millisecond precision.
Line range hint
1254-1293: LGTM: Update toListsTTLfor TTL in milliseconds.The method now retrieves TTL in milliseconds, using
pstd::NowMillis()for current time retrieval, aligning with the transition to millisecond precision.src/storage/src/redis_hashes.cc (5)
34-34: Use ofpstd::NowMillis()is appropriate.The transition to milliseconds is consistent with the PR objectives.
Line range hint
223-256:
Correct transition to milliseconds.The use of
ttl_millsecand the logic for calculating the TTL in milliseconds are correctly implemented.
Line range hint
158-188:
Correct usage ofttl_millsec.The function correctly sets the TTL in milliseconds.
Line range hint
235-264:
Correct usage oftimestamp_millsec.The function correctly sets the expiration timestamp in milliseconds.
Line range hint
314-352:
Correct usage ofttl_millsec.The function correctly calculates and returns the TTL in milliseconds.
src/storage/src/redis_strings.cc (18)
36-36: LGTM: Use ofpstd::NowMillis()for millisecond precision.The change to use
pstd::NowMillis()ensures consistency with the new time unit standard.
65-68: LGTM: Consistent use of millisecond-based timestamp.The method correctly initializes and uses
expired_timestamp_millsecin line with the new precision requirements.Also applies to: 97-97, 103-103
384-384: LGTM: Millisecond precision in TTL calculation.Using
pstd::NowMillis()for TTL calculation is consistent with the new time unit standard.
388-395: LGTM: Handling of TTL in milliseconds.The method correctly handles TTL using
ttl_millsec, aligning with the new precision requirements.
Line range hint
400-421: LGTM: Updated TTL handling with millisecond precision.The method's use of
ttl_millsecaligns with the shift to millisecond-based time handling.
Line range hint
427-441: LGTM: Consistent TTL handling with millisecond precision.The method uses
ttl_millsecappropriately, consistent with the updated time handling approach.
Line range hint
532-561: LGTM: Millisecond precision in range retrieval with TTL.The method correctly handles TTL using
ttl_millsec, ensuring consistency with the new precision requirements.Also applies to: 587-587
Line range hint
622-660: LGTM: Consistent use of millisecond-based timestamp in increment operation.The method correctly handles
expired_timestamp_millsec, aligning with the transition to millisecond precision.
Line range hint
781-814: LGTM: Millisecond precision in conditional set operation.The method's use of
ttl_millsecaligns with the shift to millisecond-based time handling.
879-884: LGTM: Millisecond precision in set with expiration.The method correctly handles TTL using
ttl_millsec, ensuring consistency with the new precision requirements.
Line range hint
894-912: LGTM: Millisecond precision in conditional set if not exists.The method's use of
ttl_millsecaligns with the shift to millisecond-based time handling.
Line range hint
922-947: LGTM: Millisecond precision in conditional set with verification.The method correctly handles TTL using
ttl_millsec, ensuring consistency with the new precision requirements.
1292-1299: LGTM: Millisecond precision in set with absolute expiration.The method correctly handles timestamps using
time_stamp_millsec_, ensuring consistency with the new precision requirements.
Line range hint
1303-1330: LGTM: Millisecond precision in string expiration.The method's use of
ttl_millsecaligns with the shift to millisecond-based time handling.
Line range hint
1370-1397: LGTM: Millisecond precision in string expiration at specific time.The method correctly handles expiration timestamps using
timestamp_millsec, ensuring consistency with the new precision requirements.
Line range hint
1445-1481: LGTM: Millisecond precision in TTL retrieval.The method correctly handles TTL using
ttl_millsec, ensuring consistency with the new precision requirements.
1573-1589: LGTM: Millisecond precision in expiration handling.The method's use of
ttl_millsecaligns with the shift to millisecond-based time handling.
1597-1613: LGTM: Millisecond precision in expiration at specific time.The method correctly handles expiration timestamps using
timestamp_millsec, ensuring consistency with the new precision requirements.src/storage/src/storage.cc (20)
141-143: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
151-153: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
156-158: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
213-218: Parameter renaming and logic update improve clarity and correctness.The change from
ttltottl_millseceffectively communicates the expected time unit, and the vector logic is correctly updated.
227-229: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
258-260: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
279-281: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
284-286: Parameter renaming improves clarity.The change from
expired_timestamp_sectoexpired_timestamp_millseceffectively communicates the expected time unit.
348-350: Parameter renaming improves clarity.The change from
expired_timestamp_sectoexpired_timestamp_millseceffectively communicates the expected time unit.
358-360: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
368-373: Parameter renaming and logic update improve clarity and correctness.The change from
timestamptotime_stamp_millsec_effectively communicates the expected time unit, and the logic for handling negative timestamps is correctly implemented.
402-404: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
630-632: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
757-759: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
889-892: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
1174-1177: Parameter renaming improves clarity.The change from
ttltottl_millseceffectively communicates the expected time unit.
1453-1457: Parameter renaming improves clarity.The change from
timestamptotimestamp_millseceffectively communicates the expected time unit.
1478-1488: New methodPTTLis correctly implemented.The method correctly retrieves and returns the TTL in milliseconds, handling status checks appropriately.
1491-1499: Conversion logic inTTLis correctly implemented.The function correctly converts TTL from milliseconds to seconds for the return value.
1795-1797: Formatting adjustments improve readability.The formatting changes enhance code readability without altering functionality.
src/storage/src/redis_sets.cc (6)
37-37: LGTM!The use of
pstd::NowMillis()to obtain the current time in milliseconds aligns with the shift to milliseconds for TTL. This change is consistent and appropriate.
Line range hint
1406-1436:
LGTM!The function signature and TTL setting logic have been correctly updated to use milliseconds.
Line range hint
1483-1512:
LGTM!The function signature and expiration time setting logic have been correctly updated to use milliseconds.
999-999: LGTM!The check for modifying the count of elements in a set is appropriate and ensures that set size overflow is prevented.
Line range hint
1562-1600:
LGTM! But verify the function usage in the codebase.The function signature and TTL calculation logic have been correctly updated to use milliseconds.
Ensure that all function calls to
SetsTTLmatch the new signature.Verification successful
Function Usage Verified
The function
SetsTTLhas been updated to useint64_t* ttl_millsec, and all occurrences in the codebase match this new signature. The usage insrc/storage/src/redis_strings.ccis consistent with the updated function definition. No further issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetsTTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SetsTTL'Length of output: 1618
Line range hint
745-778:
LGTM! But verify the function usage in the codebase.The function signature and TTL calculation logic have been correctly updated to use milliseconds.
Ensure that all function calls to
SMembersWithTTLmatch the new signature.Verification successful
Function Usage Verified: All instances of
SMembersWithTTLmatch the updated signature.The function
SMembersWithTTLis correctly used and declared across the codebase with the new signature usingint64_t* ttl_millsec. No outdated usages were found.
src/pika_cache_load_thread.ccsrc/storage/src/redis.hsrc/storage/include/storage/storage.hsrc/storage/src/storage.ccsrc/storage/src/redis_sets.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SMembersWithTTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SMembersWithTTL'Length of output: 2791
src/storage/src/redis_zsets.cc (9)
39-39: LGTM: Standardized time handling.The use of
pstd::NowMillis()for getting the current time in milliseconds is consistent with the PR objective.
110-110: LGTM: Count modification check.The logic for checking and modifying the count remains intact, ensuring core functionality is unaffected.
168-168: LGTM: Count modification check.The logic for checking and modifying the count is consistent and preserved.
263-263: LGTM: Count modification logic.The count modification logic is consistent and ensures correct functionality.
446-446: LGTM: Count modification check.The logic for checking and modifying the count is consistent and maintains the function's purpose.
Line range hint
537-571: LGTM: TTL calculation in milliseconds.The function now calculates TTL in milliseconds, and the parameter name update improves clarity.
Line range hint
1672-1702: LGTM: Use of milliseconds for TTL.The function uses
ttl_millsecto set the relative timestamp, aligning with the PR objective.
Line range hint
1748-1777: LGTM: Timestamp in milliseconds.The function now uses
timestamp_millsecto set the expiration time, improving clarity and consistency.
Line range hint
1913-1951: LGTM: TTL calculation in milliseconds.The function now calculates TTL in milliseconds, and the parameter name update improves clarity.
src/storage/tests/keys_test.cc (1)
5182-5182: LGTM! Verify the impact of the new timestamp type.The change to use
pstd::TimeTypeandpstd::NowMillis()enhances precision by using milliseconds. Ensure that this change is compatible with other parts of the codebase that interact with this test.Consider verifying that all related code correctly handles the new timestamp type.
Verification successful
The use of
pstd::TimeTypeis consistent across the codebase.The search results confirm that
pstd::TimeTypeis widely used in both test and source files, indicating its compatibility and integration within the codebase. The change inkeys_test.ccaligns with existing practices.
- No further action is needed regarding the compatibility of
pstd::TimeType.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of `pstd::TimeType` with related code. # Test: Search for usage of `pstd::TimeType` in the codebase. Expect: Compatible usages. rg --type cc --type h 'pstd::TimeType'Length of output: 65
Script:
#!/bin/bash # Description: Verify compatibility of `pstd::TimeType` with related code. # Test: Search for usage of `pstd::TimeType` in the codebase. Expect: Compatible usages. rg --type cpp --type h 'pstd::TimeType'Length of output: 2498
| @@ -21,7 +21,7 @@ TEST(StringsFilterTest, FilterTest) { | |||
|
|
|||
| int64_t ttl = 1; | |||
| StringsValue strings_value("FILTER_VALUE"); | |||
| strings_value.SetRelativeTimestamp(ttl); | |||
| strings_value.SetRelativeTimeByMillsec(ttl); | |||
There was a problem hiding this comment.
SetRelativeTimeByMillsec 把单词 By 改成 In，e 文中 "以...为单位" 的英语是 in 不是 by
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
src/storage/src/redis_strings.cc (3)
Inconsistent Usage of
GetWithTTLwithttlParameterSome instances of the
GetWithTTLfunction are still using the old parameter namettlinstead ofttl_millsec. This inconsistency needs to be addressed to ensure correct functionality.
- Update the following lines to use
ttl_millsec:
rocksdb::Status s = db->storage()->GetWithTTL(key, &value, &ttl);s_ = db_->storage()->GetWithTTL(key_, &value_, &ttl_millsec);(ensure all usages are consistent)Please review and update these instances to match the new function signature.
Analysis chain
Line range hint
400-421: LGTM: Transition to millisecond precision inGetWithTTL.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `GetWithTTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'GetWithTTL'Length of output: 57
Script:
#!/bin/bash # Search for all occurrences of the `GetWithTTL` function across the codebase. rg --no-filename 'GetWithTTL' -A 5Length of output: 3759
1597-1613: UpdateExpireatfunction calls to use millisecond precisionSeveral occurrences of the
Expireatfunction still use the old signature withttl_secinstead oftimestamp_millsec. These should be updated to ensure consistency and proper functionality:
include/pika_cache.h:rocksdb::Status Expireat(std::string& key, int64_t ttl_sec);src/pika_kv.cc:db_->cache()->Expireat(key_, time_stamp_sec_);src/pika_cache.cc:Status PikaCache::Expireat(std::string& key, int64_t ttl_sec)Please update these instances to use
timestamp_millsecfor millisecond precision.Analysis chain
LGTM: Transition to millisecond precision in
Expireat.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Expireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Expireat'Length of output: 55
Script:
#!/bin/bash # Description: Verify all function calls to `Expireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Expireat'Length of output: 14537
Inconsistent Usage of
SetnxFunction SignatureThe
Setnxfunction is being used inconsistently across the codebase. Some calls include thettl_millsecparameter, while others do not. This inconsistency could lead to unexpected behavior or errors. Please ensure allSetnxfunction calls are updated to match the new signature with thettl_millsecparameter.
- File:
src/pika_kv.cc
- Line:
s_ = db_->storage()->Setnx(key_, value_, &success_);(missingttl_millsec)Please review and update the code to maintain consistency.
Analysis chain
Line range hint
894-912: LGTM: Transition to millisecond precision inSetnx.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Setnx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Setnx'Length of output: 52
Script:
#!/bin/bash # Description: Verify all function calls to `Setnx` match the new signature. # Test: Search for the function usage in .cc files. Expect: Only occurrences of the new signature. rg 'Setnx' --glob '*.cc' -A 5Length of output: 16882
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/storage/src/base_value_format.h (4 hunks)
- src/storage/src/redis_strings.cc (26 hunks)
- src/storage/tests/lists_filter_test.cc (2 hunks)
- src/storage/tests/strings_filter_test.cc (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/storage/src/base_value_format.h
- src/storage/tests/lists_filter_test.cc
- src/storage/tests/strings_filter_test.cc
Additional comments not posted (16)
src/storage/src/redis_strings.cc (16)
36-36: LGTM: Use ofpstd::NowMillis()for millisecond precision.The change from
rocksdb::Env::Default()->GetCurrentTime(&curtime)topstd::NowMillis()is consistent with the PR's goal of using milliseconds for time calculations.
384-384: LGTM: Millisecond precision inCalculateTTL.The use of
pstd::NowMillis()ensures that TTL calculations are performed with millisecond precision, aligning with the PR's objective.
Line range hint
532-587: LGTM: Millisecond precision inGetrangeWithValue.The changes to handle TTL in milliseconds are consistent with the PR's objective. Ensure that all usages of this function are updated to match the new logic.
388-395: LGTM: Millisecond precision inHandleParsedStringsValue.The changes to handle TTL in milliseconds are consistent with the PR's objective. Ensure that all usages of this function are updated to match the new logic.
Verification successful
Verification Complete: Function
HandleParsedStringsValueUsage is ConsistentThe function
HandleParsedStringsValueis used consistently with the new logic withinsrc/storage/src/redis_strings.cc. No discrepancies were found in its usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `HandleParsedStringsValue` match the new logic. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type cc -A 5 $'HandleParsedStringsValue'Length of output: 71
Script:
#!/bin/bash # Description: Verify all function calls to `HandleParsedStringsValue` match the new logic. # Test: Search for the function usage in .cc files. rg 'HandleParsedStringsValue' --glob '*.cc' -A 5Length of output: 1296
Line range hint
781-814: LGTM: Transition to millisecond precision inSetxx.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Function
Setxxusage is consistent with the new signature.The function
Setxxis used with the updated signature, including thettl_millsecparameter, across various files in the codebase. This confirms that the transition to millisecond precision is correctly implemented.
- Files with updated usage:
src/pika_kv.ccsrc/storage/src/storage.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Setxx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Setxx'Length of output: 52
Script:
#!/bin/bash # Description: Verify all function calls to `Setxx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Setxx'Length of output: 8800
Line range hint
922-947: LGTM: Transition to millisecond precision inSetvx.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Transition to millisecond precision for
Setvxis consistent.The
Setvxfunction is being used with the newttl_millsecparameter across the codebase, including in tests. This indicates that the transition to millisecond precision is well-implemented. No further action is needed.
- Verified usage in
src/pika_kv.cc,src/storage/src/storage.cc, andsrc/storage/tests/strings_test.cc.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Setvx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Setvx'Length of output: 52
Script:
#!/bin/bash # Description: Verify all function calls to `Setvx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Setvx'Length of output: 5299
Line range hint
427-441: LGTM: Transition to millisecond precision inMGetWithTTL.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Verification Successful: Consistent Update to
MGetWithTTLFunction SignatureThe function
MGetWithTTLhas been updated to useint64_t* ttl_millsecconsistently across its definitions and calls in the codebase. The transition to millisecond precision is correctly implemented.
- File:
src/pika_kv.cc- File:
src/storage/src/redis_strings.cc- File:
src/storage/src/storage.cc- File:
src/storage/include/storage/storage.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `MGetWithTTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'MGetWithTTL'Length of output: 58
Script:
#!/bin/bash # Description: Verify all function calls to `MGetWithTTL` match the new signature. # Corrected Test: Search for the function usage in C++ files. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'MGetWithTTL'Length of output: 3692
Line range hint
1303-1330: LGTM: Transition to millisecond precision inStringsExpire.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
All usages of
StringsExpireare consistent with the new signature.The function
StringsExpirehas been correctly updated to usettl_millsecas a parameter, and all usages in the codebase reflect this change. No further updates are needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StringsExpire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'StringsExpire'Length of output: 60
Script:
#!/bin/bash # Description: Verify all function calls to `StringsExpire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'StringsExpire'Length of output: 3017
Line range hint
1445-1481: LGTM: Transition to millisecond precision inStringsTTL.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Function
StringsTTLusage verified with new signatureThe search results confirm that the function
StringsTTLis being used with the updated signature across the codebase. The changes align with the transition to millisecond precision for TTL handling.
- File:
src/storage/src/redis_strings.cc
- Line:
return StringsTTL(key, ttl_millsec, std::move(meta_value));- File:
src/storage/src/redis.h
- Line:
virtual Status StringsTTL(const Slice& key, int64_t* ttl_millsec, std::string&& prefetch_meta = {});These instances reflect the correct usage of the updated function signature. No further action is required.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StringsTTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'StringsTTL'Length of output: 57
Script:
#!/bin/bash # Description: Verify all function calls to `StringsTTL` match the new signature. # Correct file type and rerun the search. rg --type cpp -A 5 $'StringsTTL'Length of output: 1478
879-884: LGTM: Transition to millisecond precision inSetex.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Transition to millisecond precision in
Setexis consistent across the codebase.All identified usages of the
Setexfunction have been updated to reflect the new signature withttl_millsec. This includes updates in the main code, tests, and examples.
- Files with updated
Setexcalls:
src/pika_kv.ccsrc/storage/tests/strings_test.ccsrc/storage/examples/strings_example.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Setex` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Setex'Length of output: 52
Script:
#!/bin/bash # Description: Verify all function calls to `Setex` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Setex'Length of output: 16588
Line range hint
622-660: LGTM: Transition to millisecond precision inIncrby.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Verification Successful: Transition to Millisecond Precision Confirmed
The
Incrbyfunction is consistently used with the updated signature, including theexpired_timestamp_millsecparameter, across the codebase. The transition to millisecond precision for expiration timestamps is correctly implemented.
- Files such as
src/pika_kv.ccandsrc/storage/tests/strings_test.ccshow correct usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Incrby` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Incrby'Length of output: 53
Script:
#!/bin/bash # Description: Verify all function calls to `Incrby` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Incrby'Length of output: 65161
1292-1299: LGTM: Millisecond precision inPKSetexAt.The changes to handle timestamps in milliseconds are consistent with the PR's objective. Ensure that all usages of this function are updated to match the new logic.
Verification successful
Function Usage Consistent with Millisecond Precision
The
PKSetexAtfunction is used consistently across the codebase with the updated logic for handling timestamps in milliseconds. The test cases confirm that the changes are verified. No further action is required.
pika_kv.cc: Correct usage with millisecond conversion.strings_test.cc: Test cases validate the function's behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `PKSetexAt` match the new logic. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type cc -A 5 $'PKSetexAt'Length of output: 56
Script:
#!/bin/bash # Description: Verify all function calls to `PKSetexAt` match the new logic. # Test: Search for the function usage in C++ files. Expect: Only occurrences of the new logic. rg --glob '*.cc' -A 5 'PKSetexAt'Length of output: 6042
Line range hint
1370-1397: LGTM: Transition to millisecond precision inStringsExpireat.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Function signature update verified:
StringsExpireatusage is consistent.The function
StringsExpireatis correctly called with the updated signature (key,timestamp_millsec,std::move(meta_value)) withinsrc/storage/src/redis_strings.cc. No other usages were found, confirming consistency.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StringsExpireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'StringsExpireat'Length of output: 62
Script:
#!/bin/bash # Description: Verify all function calls to `StringsExpireat` match the new signature. # Test: Search for the function usage in .cc files. rg 'StringsExpireat' --glob '*.cc' -A 5Length of output: 852
1573-1589: LGTM: Transition to millisecond precision inExpire.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
1645-1661: LGTM: Transition to millisecond precision inTTL.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Verification Successful: Consistent Usage of
TTLwith Millisecond PrecisionThe
TTLfunction has been updated to usettl_millsecconsistently across the codebase. The transition to millisecond precision is correctly reflected in all relevant function calls.
- No inconsistencies found in the usage of the
TTLfunction.- All instances align with the updated function signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `TTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'TTL'Length of output: 50
Script:
#!/bin/bash # Description: Verify all function calls to `TTL` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'TTL'Length of output: 67961
Line range hint
65-103: LGTM: Transition to millisecond precision inAppend.The parameter renaming and logic adjustments align with the shift to millisecond-based time handling. Ensure that all usages of this function are updated to match the new signature.
Verification successful
Verification Complete: Transition to Millisecond Precision in
AppendThe function
Appendis used consistently with the updated signature in both the source and test files, confirming the successful transition to millisecond-based time handling.
- Verified in
src/storage/src/storage.ccandsrc/storage/tests/strings_test.cc.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Append` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cc -A 5 $'Append'Length of output: 53
Script:
#!/bin/bash # Description: Verify all function calls to `Append` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Append'Length of output: 193040
Script:
#!/bin/bash # Description: Verify specific occurrences of the `Append` function in relevant files. # Test: Search for the function usage in `storage.cc` and `strings_test.cc`. rg --type cpp -A 5 'Append' src/storage/src/storage.cc src/storage/tests/strings_test.ccLength of output: 3678
…undation#2857) * fix: change storage ttl time from seconds to milliseconds (OpenAtomFoundation#2822) * change storage ttl time from seconds to milliseconds --------- Co-authored-by: liuyuecai <[email protected]> test space * fix incr cmd time to millionsseconds * rename SetRelativeTimeByMillsec to SetRelativeTimeInMillsec --------- Co-authored-by: liuyuecai <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style