fix: change storage ttl time from seconds to milliseconds #2822
Conversation
luky116
changed the title
WalkthroughThe changes enhance the time-to-live (TTL) command functionality by transitioning from a second-based approach to a more precise millisecond-based method with the introduction of the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- src/pika_kv.cc (2 hunks)
- src/storage/include/storage/storage.h (1 hunks)
- src/storage/src/base_filter.h (2 hunks)
- src/storage/src/base_meta_value_format.h (1 hunks)
- src/storage/src/base_value_format.h (3 hunks)
- src/storage/src/lists_filter.h (1 hunks)
- src/storage/src/lists_meta_value_format.h (2 hunks)
- src/storage/src/redis_hashes.cc (3 hunks)
- src/storage/src/redis_lists.cc (3 hunks)
- src/storage/src/redis_sets.cc (3 hunks)
- src/storage/src/redis_streams.cc (1 hunks)
- src/storage/src/redis_strings.cc (4 hunks)
- src/storage/src/redis_zsets.cc (3 hunks)
- src/storage/src/storage.cc (4 hunks)
- src/storage/src/strings_filter.h (1 hunks)
- src/storage/src/zsets_filter.h (1 hunks)
- src/storage/tests/keys_test.cc (1 hunks)
- src/storage/tests/strings_test.cc (1 hunks)
Files skipped from review due to trivial changes (5)
- src/storage/src/base_meta_value_format.h
- src/storage/src/redis_hashes.cc
- src/storage/src/redis_streams.cc
- src/storage/src/redis_strings.cc
- src/storage/tests/strings_test.cc
Additional comments not posted (25)
src/storage/src/strings_filter.h (1)
23-23: LGTM! Verify the usage ofNowMicrosacross the codebase.The change to use
NowMicrosfor more precise time measurement is approved.Ensure that
NowMicrosis used consistently across the codebase for time-related operations.Verification successful
LGTM! Verify the usage of
NowMicrosacross the codebase.The change to use
NowMicrosfor more precise time measurement is approved.Ensure that
NowMicrosis used consistently across the codebase for time-related operations. The provided context shows consistent usage, but a thorough review across all instances is recommended.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NowMicros` across the codebase. # Test: Search for the usage of `NowMicros`. Expect: Consistent usage across the codebase. rg --type cpp -A 5 'NowMicros'Length of output: 38604
src/storage/src/base_value_format.h (2)
125-126: LGTM! Verify the usage ofNowMicrosacross the codebase.The change to use
NowMicrosfor more precise timestamp calculation is approved.Ensure that
NowMicrosis used consistently across the codebase for time-related operations.Verification successful
Consistent Usage of
NowMicrosVerifiedThe usage of
NowMicrosacross the codebase is consistent and correct, often converting microseconds to milliseconds where appropriate. The change inbase_value_format.haligns well with the rest of the codebase.
- Verified consistent usage of
NowMicrosacross various files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NowMicros` across the codebase. # Test: Search for the usage of `NowMicros`. Expect: Consistent usage across the codebase. rg --type cpp -A 5 'NowMicros'Length of output: 38604
55-56: LGTM! Verify the usage ofNowMicrosacross the codebase.The change to use
NowMicrosfor more precise timestamp calculation is approved.Ensure that
NowMicrosis used consistently across the codebase for time-related operations.Verification successful
LGTM! The usage of
NowMicrosis consistent across the codebase.The change to use
NowMicrosfor more precise timestamp calculation is approved. The function is used consistently for time-related operations across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NowMicros` across the codebase. # Test: Search for the usage of `NowMicros`. Expect: Consistent usage across the codebase. rg --type cpp -A 5 'NowMicros'Length of output: 38604
src/storage/src/zsets_filter.h (1)
83-83: LGTM! Verify the usage ofNowMicrosacross the codebase.The change to use
NowMicrosfor more precise time measurement is approved.Ensure that
NowMicrosis used consistently across the codebase for time-related operations.Verification successful
Verified: Consistent usage of
NowMicrosacross the codebase.The
NowMicrosfunction is used consistently across various files for obtaining the current time in microseconds, often divided by 1000 to convert to milliseconds. The change inzsets_filter.haligns with this usage pattern.
- src/pika_auxiliary_thread.cc
- src/pika_client_conn.cc
- src/pika_repl_client_conn.cc
- src/pika_rm.cc
- src/throttle.cc
- src/rsync_client.cc
- src/pika_statistic.cc
- src/storage/src/base_value_format.h
- src/storage/src/redis_hashes.cc
- src/storage/src/base_filter.h
- src/storage/src/strings_filter.h
- src/storage/src/redis_lists.cc
- src/storage/src/lists_filter.h
- src/storage/src/base_meta_value_format.h
- src/storage/src/redis_zsets.cc
- src/storage/src/lists_meta_value_format.h
- src/storage/src/redis.h
- src/pika_repl_server_conn.cc
- src/pika_server.cc
- src/pika_repl_bgworker.cc
- src/pika_db.cc
- src/pika_consensus.cc
- src/pika_command.cc
- src/pika_cache_load_thread.cc
- src/net/examples/thread_pool_test.cc
- src/net/examples/performance/server.cc
- tools/benchmark_client/benchmark_client.cc
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NowMicros` across the codebase. # Test: Search for the usage of `NowMicros`. Expect: Consistent usage across the codebase. rg --type cpp -A 5 'NowMicros'Length of output: 38604
src/storage/src/lists_filter.h (1)
91-91: LGTM! The current time retrieval is now simplified and consistent.The change updates the method of retrieving the current time to use milliseconds instead of seconds, which simplifies the code and ensures consistency.
src/storage/src/lists_meta_value_format.h (2)
54-54: LGTM! The current time retrieval is now simplified and consistent.The change updates the method of retrieving the current time to use milliseconds instead of seconds, which simplifies the code and ensures consistency.
200-200: LGTM! The current time retrieval is now simplified and consistent.The change updates the method of retrieving the current time to use milliseconds instead of seconds, which simplifies the code and ensures consistency.
src/storage/src/base_filter.h (2)
31-31: LGTM! The current time retrieval is now simplified and consistent.The change updates the method of retrieving the current time to use milliseconds instead of seconds, which simplifies the code and ensures consistency.
182-182: LGTM! The current time retrieval is now simplified and consistent.The change updates the method of retrieving the current time to use milliseconds instead of seconds, which simplifies the code and ensures consistency.
src/pika_kv.cc (2)
1445-1446: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
ReadCachematch the new implementation.Verification successful
Verification Successful for
PttlCmd::ReadCacheThe
ReadCachefunction in thePttlCmdclass has been correctly updated to callDoThroughDBdirectly, as Redis cache does not support PTTL cache. This ensures consistency in PTTL handling.However, it is recommended to verify other command classes to ensure they follow the expected pattern of cache usage.
GetCmd::ReadCacheMgetCmd::ReadCacheGetrangeCmd::ReadCacheStrlenCmd::ReadCacheExistsCmd::ReadCacheTtlCmd::ReadCacheTypeCmd::ReadCacheZCardCmd::ReadCacheZRangeCmd::ReadCacheZRevrangeCmd::ReadCacheZRangebyscoreCmd::ReadCacheZRevrangebyscoreCmd::ReadCacheZCountCmd::ReadCacheZRankCmd::ReadCacheZRevrankCmd::ReadCacheZScoreCmd::ReadCacheZRangebylexCmd::ReadCacheZRevrangebylexCmd::ReadCacheZLexcountCmd::ReadCacheSCardCmd::ReadCacheSMembersCmd::ReadCacheSIsmemberCmd::ReadCacheSRandmemberCmd::ReadCacheHGetCmd::ReadCacheHGetallCmd::ReadCacheHExistsCmd::ReadCacheHKeysCmd::ReadCacheHLenCmd::ReadCacheHMgetCmd::ReadCacheHStrlenCmd::ReadCacheHValsCmd::ReadCacheLIndexCmd::ReadCacheLLenCmd::ReadCacheLRangeCmd::ReadCacheRPopLPushCmd::ReadCacheBitGetCmd::ReadCacheBitCountCmd::ReadCacheBitPosCmd::ReadCacheScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ReadCache` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type cpp -A 5 $'ReadCache'Length of output: 44694
Line range hint
1436-1440:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
PTTLmatch the new implementation.Verification successful
The function
PTTLis used in the following files:
src/pika_kv.ccsrc/storage/include/storage/storage.hsrc/storage/src/storage.ccThe implementation and usage of
PTTLinsrc/storage/src/storage.ccneed to be reviewed to ensure they align with the new changes.
Verification successful: The
PTTLfunction implementation aligns with the new changes to use milliseconds for TTL values. The function is correctly implemented and handles errors appropriately.
src/storage/src/storage.cc: Verified implementation ofPTTL.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `PTTL` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type cpp -A 5 $'PTTL'Length of output: 2142
Script:
#!/bin/bash # Description: Review the implementation of the `PTTL` function in `src/storage/src/storage.cc`. # Search for the implementation of the `PTTL` function in the file. rg --type cpp -A 10 $'int64_t Storage::PTTL(const Slice& key)'Length of output: 63
Script:
#!/bin/bash # Description: Re-verify the implementation of the `PTTL` function in the entire codebase. # Search for the implementation of the `PTTL` function in the entire codebase. rg --type cpp -A 10 $'int64_t PTTL(const Slice& key)'Length of output: 54
Script:
#!/bin/bash # Description: Re-verify the implementation of the `PTTL` function in the entire codebase. # Search for the implementation of the `PTTL` function in the entire codebase without specifying the return type. rg --type cpp -A 10 $'PTTL(const Slice& key)'Length of output: 46
Script:
#!/bin/bash # Description: Re-verify the implementation of the `PTTL` function in the entire codebase. # Search for all function definitions in the codebase and filter for `PTTL`. rg --type cpp --pcre2 -A 10 '\bPTTL\b' --glob '*.*'Length of output: 5588
src/storage/include/storage/storage.h (2)
1037-1037: Typographical correction approved.The correction from "Reutrns" to "Returns" is accurate and does not affect the logic or functionality of the code.
1030-1035: Verify the implementation of thePTTLmethod.The method signature and comments look correct. Ensure that the implementation of this method in the corresponding
.cppfile aligns with the described behavior:
- Returns -3 for operation exceptions.
- Returns -2 if the key does not exist.
- Returns -1 if the key exists but has no associated expiration.
- Returns a positive value representing the TTL in milliseconds if the key is valid.
src/storage/src/redis_lists.cc (3)
33-33: Efficient time retrieval.The change to use
rocksdb::Env::Default()->NowMicros() / 1000for retrieving the current time in milliseconds is more efficient and simplifies the logic.
505-505: Efficient time retrieval.The change to use
rocksdb::Env::Default()->NowMicros() / 1000for retrieving the current time in milliseconds is more efficient and simplifies the logic.
1288-1288: Efficient time retrieval.The change to use
rocksdb::Env::Default()->NowMicros() / 1000for retrieving the current time in milliseconds is more efficient and simplifies the logic.src/storage/src/storage.cc (2)
Line range hint
1471-1481:
Verify the function usage in the codebase.The
PTTLfunction has been renamed fromTTLand modified to return the timestamp directly in milliseconds. Ensure that all function calls toPTTLmatch the new behavior.Verification successful
Verify the function usage in the codebase.
The function
PTTLhas been correctly updated and used in the codebase. The new behavior is consistent, and the function calls handle the returned timestamp appropriately.
src/pika_kv.cc: The function call toPTTLchecks for the-3error code and appends the timestamp to the response.src/storage/include/storage/storage.h: The function declaration matches the new behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `PTTL` match the new behavior. # Test: Search for the function usage. Expect: Only occurances of the new behavior. rg --type cpp -A 5 $'PTTL'Length of output: 2142
1483-1493: Verify the function usage in the codebase.The
TTLfunction has been introduced to handle TTL values in seconds, converting milliseconds to seconds. Ensure that all function calls toTTLmatch the new behavior.src/storage/src/redis_sets.cc (3)
37-37: LGTM!The change to retrieve the current time in milliseconds aligns with the objective of transitioning to millisecond precision.
777-777: LGTM!The change to retrieve the current time in milliseconds aligns with the objective of transitioning to millisecond precision.
1597-1597: LGTM!The change to retrieve the current time in milliseconds aligns with the objective of transitioning to millisecond precision.
src/storage/src/redis_zsets.cc (3)
39-39: Improved readability and conciseness.The change to retrieve the current time in milliseconds directly improves readability and reduces boilerplate code.
570-570: Improved readability and conciseness.The change to retrieve the current time in milliseconds directly improves readability and reduces boilerplate code.
1946-1946: Improved readability and conciseness.The change to retrieve the current time in milliseconds directly improves readability and reduces boilerplate code.
src/storage/tests/keys_test.cc (1)
5116-5118: LGTM! Simplified time retrieval.The change simplifies the code by directly retrieving the current time in microseconds and converting it to milliseconds. This reduces the number of function calls and potentially improves performance. The functionality remains unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pika_kv.cc (2 hunks)
Additional comments not posted (2)
src/pika_kv.cc (2)
1445-1446: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
ReadCachematch the new usage.
Line range hint
1436-1440:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
PTTLmatch the new usage.Verification successful
Verification successful: All function calls to
PTTLmatch the new usage.
- The
PttlCmd::Do()method correctly usesPTTLto retrieve the TTL in milliseconds.- The
PttlCmd::ReadCache()method does not interact with the cache and directly reads from the database, ensuring no issues with cache handling.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `PTTL` match the new usage. # Test: Search for the function usage. Expect: Only occurrences of the new usage. rg --type cpp -A 5 $'PTTL'Length of output: 2142
Script:
#!/bin/bash # Description: Verify the implementation of `ReadCache()` method in `src/pika_kv.cc`. # Test: Search for the `ReadCache` method definition in the file. rg --type cpp -A 10 $'ReadCache' src/pika_kv.ccLength of output: 2193
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/storage/src/base_meta_value_format.h (2 hunks)
- src/storage/src/lists_meta_value_format.h (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/storage/src/base_meta_value_format.h
- src/storage/src/lists_meta_value_format.h
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
src/storage/src/storage.cc (1)
1471-1475: Document the error code-3.For better readability and maintainability, document the meaning of the error code
-3returned by this method.// Returns -3 if the key is not found or an error occurs.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/src/storage.cc (3 hunks)
Additional comments not posted (2)
src/storage/src/storage.cc (2)
1481-1489: LGTM!The
TTLmethod is correctly implemented and handles the conversion from milliseconds to seconds properly.
1784-1786: LGTM!The
SetSmallCompactionThresholdmethod is correctly implemented.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/storage/src/base_meta_value_format.h (2 hunks)
- src/storage/src/lists_meta_value_format.h (2 hunks)
- src/storage/src/storage.cc (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/storage/src/base_meta_value_format.h
- src/storage/src/lists_meta_value_format.h
- src/storage/src/storage.cc
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pika.yml (2 hunks)
Additional comments not posted (2)
.github/workflows/pika.yml (2)
57-57: Enhance test output verbosity.Adding the
--verboseflag to thectestcommand will provide more detailed output during the testing process, which can be beneficial for debugging and understanding test results.
114-114: Enhance test output verbosity.Adding the
--verboseflag to thectestcommand will provide more detailed output during the testing process, which can be beneficial for debugging and understanding test results.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/tests/strings_test.cc (4 hunks)
Additional comments not posted (6)
src/storage/tests/strings_test.cc (6)
948-949: Ensure accurate time conversion.The
unix_timecalculation now usesrocksdb::Env::Default()->NowMicros() / 1000to convert microseconds to milliseconds. This change ensures higher precision in time handling.
959-962: Ensure accurate time conversion.The
unix_timecalculation now usesrocksdb::Env::Default()->NowMicros() / 1000to convert microseconds to milliseconds. This change ensures higher precision in time handling.
972-973: Ensure accurate time conversion.The
unix_timecalculation now usesrocksdb::Env::Default()->NowMicros() / 1000to convert microseconds to milliseconds. This change ensures higher precision in time handling.
981-984: Ensure accurate time conversion.The
unix_timecalculation now usesrocksdb::Env::Default()->NowMicros() / 1000to convert microseconds to milliseconds. This change ensures higher precision in time handling.
992-992: Ensure accurate time conversion.The
unix_timecalculation now usesrocksdb::Env::Default()->NowMicros() / 1000to convert microseconds to milliseconds. This change ensures higher precision in time handling.
1001-1001: Ensure accurate time conversion.The
unix_timecalculation now usesrocksdb::Env::Default()->NowMicros() / 1000to convert microseconds to milliseconds. This change ensures higher precision in time handling.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/src/redis_hashes.cc (4 hunks)
Additional comments not posted (4)
src/storage/src/redis_hashes.cc (4)
34-34: LGTM! Improved time retrieval method.The change to use
rocksdb::Env::Default()->NowMicros() / 1000for retrieving the current time improves precision and performance.
255-255: LGTM! Improved time retrieval method.The change to use
rocksdb::Env::Default()->NowMicros() / 1000for retrieving the current time improves precision and performance.
1346-1346: LGTM! Improved time retrieval method.The change to use
rocksdb::Env::Default()->NowMicros() / 1000for retrieving the current time improves precision and performance.
1122-1122: LGTM! Improved type safety for version handling.The change to use
uint64_tforstart_key_versionimproves type safety and allows for a broader range of values without overflow risk.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/pstd/include/env.h (1 hunks)
- src/pstd/src/env.cc (1 hunks)
- src/storage/tests/keys_test.cc (1 hunks)
- src/storage/tests/strings_test.cc (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/storage/tests/strings_test.cc
Additional comments not posted (3)
src/pstd/include/env.h (1)
66-67: LGTM! The function declaration is clear and consistent.The
NowMillisfunction declaration is correct and aligns well with the existingNowMicrosfunction.src/pstd/src/env.cc (1)
225-229: LGTM! The function implementation is correct and efficient.The
NowMillisfunction correctly captures the current time and converts it to milliseconds usingstd::chrono.src/storage/tests/keys_test.cc (1)
5116-5119: Simplified time retrieval process aligns with PR objectives.The change to use
pstd::NowMillis()for getting the current time in milliseconds simplifies the code and aligns with the objective of handling TTL in milliseconds. The functional outcome remains consistent.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- src/storage/src/base_filter.h (2 hunks)
- src/storage/src/base_meta_value_format.h (2 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 (2 hunks)
- src/storage/src/redis_hashes.cc (4 hunks)
- src/storage/src/redis_lists.cc (3 hunks)
- src/storage/src/redis_sets.cc (4 hunks)
- src/storage/src/redis_strings.cc (4 hunks)
- src/storage/src/redis_zsets.cc (3 hunks)
- src/storage/src/strings_filter.h (1 hunks)
- src/storage/src/zsets_filter.h (1 hunks)
Files skipped from review due to trivial changes (1)
- src/storage/src/zsets_filter.h
Files skipped from review as they are similar to previous changes (1)
- src/storage/src/redis_hashes.cc
Additional comments not posted (25)
src/storage/src/strings_filter.h (1)
23-23: LGTM! Ensure consistency with the rest of the codebase.The change to use
pstd::NowMillis()for obtaining the current time in milliseconds is correct. Ensure this method is consistently used throughout the codebase for time retrieval.src/storage/src/base_value_format.h (4)
44-44: LGTM!The change to use
pstd::NowMillis()for initializingctime_in the constructor is correct.
55-56: LGTM!The change to use
pstd::NowMillis()and convertttlto milliseconds in theSetRelativeTimestampmethod is correct.
125-126: LGTM!The change to use
pstd::NowMillis()and convertttlto milliseconds in theSetRelativeTimestampmethod is correct.
136-136: LGTM!The change to use
pstd::NowMillis()for obtaining the current time in milliseconds in theIsStalemethod is correct.src/storage/src/lists_filter.h (1)
91-91: LGTM! Ensure consistency with the rest of the codebase.The change to use
pstd::NowMillis()for obtaining the current time in milliseconds is correct. Ensure this method is consistently used throughout the codebase for time retrieval.src/storage/src/base_meta_value_format.h (2)
Line range hint
49-53:
LGTM!The
UpdateVersionmethod now usespstd::NowMillis()to retrieve the current time in milliseconds, which aligns with the PR objectives.
Line range hint
174-180:
LGTM!The
UpdateVersionmethod now usespstd::NowMillis()to retrieve the current time in milliseconds, which aligns with the PR objectives.src/storage/src/lists_meta_value_format.h (2)
Line range hint
54-60:
LGTM!The
UpdateVersionmethod now usespstd::NowMillis()to retrieve the current time in milliseconds, which aligns with the PR objectives.
Line range hint
200-206:
LGTM!The
UpdateVersionmethod now usespstd::NowMillis()to retrieve the current time in milliseconds, which aligns with the PR objectives.src/storage/src/base_filter.h (2)
31-33: LGTM!The
Filtermethod now usespstd::NowMillis()to retrieve the current time in milliseconds, which aligns with the PR objectives.
182-184: LGTM!The
Filtermethod now usespstd::NowMillis()to retrieve the current time in milliseconds, which aligns with the PR objectives.src/storage/src/redis_lists.cc (3)
33-33: LGTM! The change improves performance and clarity.Replacing
rocksdb::Env::Default()->GetCurrentTime(&curtime)withcurtime = pstd::NowMillis()simplifies the code and may improve performance by reducing overhead.
505-505: LGTM! The change improves performance and clarity.Replacing
rocksdb::Env::Default()->GetCurrentTime(&curtime)withcurtime = pstd::NowMillis()simplifies the code and may improve performance by reducing overhead.
1288-1288: LGTM! The change improves performance and clarity.Replacing
rocksdb::Env::Default()->GetCurrentTime(&curtime)withcurtime = pstd::NowMillis()simplifies the code and may improve performance by reducing overhead.src/storage/src/redis_strings.cc (4)
36-36: LGTM!The change to use
pstd::NowMillis()improves readability and potentially performance.
380-380: LGTM!The change to use
pstd::NowMillis()improves readability and potentially performance.
556-556: LGTM!The change to use
pstd::NowMillis()improves readability and potentially performance.
1469-1469: LGTM!The change to use
pstd::NowMillis()improves readability and potentially performance.src/storage/src/redis_sets.cc (3)
37-37: LGTM! The change aligns with the objective of handling TTL values in milliseconds.The replacement of
rocksdb::Env::Default()->GetCurrentTime(&curtime)withcurtime = pstd::NowMillis()is appropriate and aligns with the PR objectives.
777-777: LGTM! The change aligns with the objective of handling TTL values in milliseconds.The replacement of
rocksdb::Env::Default()->GetCurrentTime(&curtime)withcurtime = pstd::NowMillis()is appropriate and aligns with the PR objectives.
1595-1595: LGTM! The change aligns with the objective of handling TTL values in milliseconds.The replacement of
rocksdb::Env::Default()->GetCurrentTime(&curtime)withcurtime = pstd::NowMillis()is appropriate and aligns with the PR objectives.src/storage/src/redis_zsets.cc (3)
39-39: LGTM! Simplified time retrieval.The change simplifies the code by directly obtaining the current time in milliseconds using
pstd::NowMillis(). This aligns with the PR objective of handling TTL values in milliseconds.
570-572: LGTM! Simplified time retrieval.The change simplifies the code by directly obtaining the current time in milliseconds using
pstd::NowMillis(). This aligns with the PR objective of handling TTL values in milliseconds.
1946-1948: LGTM! Simplified time retrieval.The change simplifies the code by directly obtaining the current time in milliseconds using
pstd::NowMillis(). This aligns with the PR objective of handling TTL values in milliseconds.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- src/pika_admin.cc (2 hunks)
- src/pika_bit.cc (1 hunks)
- src/pika_conf.cc (2 hunks)
- src/pika_geo.cc (1 hunks)
- src/pika_kv.cc (6 hunks)
- src/pika_list.cc (1 hunks)
- src/pika_server.cc (5 hunks)
- src/pika_zset.cc (5 hunks)
- src/storage/src/base_meta_value_format.h (5 hunks)
- src/storage/src/lists_meta_value_format.h (5 hunks)
- src/storage/src/redis_hashes.cc (10 hunks)
- src/storage/src/redis_sets.cc (9 hunks)
- src/storage/src/redis_strings.cc (5 hunks)
- src/storage/src/redis_zsets.cc (11 hunks)
- src/storage/src/storage.cc (6 hunks)
- src/storage/src/strings_value_format.h (4 hunks)
Files skipped from review due to trivial changes (7)
- src/pika_admin.cc
- src/pika_bit.cc
- src/pika_conf.cc
- src/pika_geo.cc
- src/pika_list.cc
- src/pika_server.cc
- src/pika_zset.cc
Files skipped from review as they are similar to previous changes (8)
- src/storage/src/base_meta_value_format.h
- src/storage/src/lists_meta_value_format.h
- src/storage/src/redis_hashes.cc
- src/storage/src/redis_sets.cc
- src/storage/src/redis_strings.cc
- src/storage/src/redis_zsets.cc
- src/storage/src/storage.cc
- src/storage/src/strings_value_format.h
Additional comments not posted (6)
src/pika_kv.cc (6)
494-494: LGTM!The change to directly call the
Domethod inAppendCmd::DoThroughDBis straightforward and maintains the existing logic.
1296-1296: LGTM!The change to directly call the
Domethod inPexpireCmd::DoThroughDBis straightforward and maintains the existing logic.
532-532: LGTM!The change to handle cases where
cache_miss_keys_is empty inMgetCmd::Dois logical and ensures proper functionality.
947-947: LGTM!The change to iterate over
kvs_to generate binlog entries inMsetCmd::DoBinlogis logical and ensures proper functionality.
1436-1436: LGTM!The change to use the
PTTLmethod to retrieve the time-to-live value inPttlCmd::Dois logical and ensures proper functionality.
1445-1446: LGTM!The change to directly call
DoThroughDBto read from the database inPttlCmd::ReadCacheis logical and ensures proper functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
src/storage/src/redis.h (1)
188-188: Update required: Instances ofExpireusing the old signatureThere are still instances of the
Expiremethod using the old signature withttlinstead ofttl_millsec. Please update the following occurrences to use the new signature withttl_millsec:
src/pika_cache.cc
- Line 1:
Status PikaCache::Expire(std::string& key, int64_t ttl)- Line 2:
return caches_[cache_index]->Expire(key, ttl)- Additional lines: Multiple occurrences within the file
src/cache/include/cache.h
- Line 1:
Status Expire(std::string& key, int64_t ttl)- Line 2:
Status Expireat(std::string& key, int64_t ttl)
src/cache/src/cache.cc
- Line 1:
Status RedisCache::Expire(std::string& key, int64_t ttl)- Line 2:
Status RedisCache::Expireat(std::string& key, int64_t ttl)Please ensure that all instances are updated accordingly.
Analysis chain
LGTM! But verify the method usage in the codebase.
The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toExpireare updated accordingly.
Verify all instances of
Expirehave been updated to usettl_millsec.The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. However, there are still some instances in the codebase that might be using the old signature. Let's verify and ensure that all calls toExpireare updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Expire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Expire'Length of output: 53310
Script:
#!/bin/bash # Description: Verify all function calls to `Expire` match the new signature with `ttl_millsec`. # Test: Search for the function usage with the old signature. Expect: No occurrences of the old signature. rg --type cpp -A 5 $'Expire(.*ttl[^_])'Length of output: 3304
src/storage/include/storage/storage.h (1)
963-963: Update all instances ofExpireto use the new signatureThe change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. However, there are still instances in the codebase where the old signature is being used. Ensure that all calls toExpireare updated accordingly.
- Files to update:
src/storage/tests/strings_test.ccsrc/storage/tests/keys_test.ccsrc/storage/tests/sets_test.ccsrc/storage/tests/zsets_test.ccsrc/storage/tests/lists_test.ccsrc/storage/tests/hashes_test.ccsrc/storage/examples/strings_example.ccsrc/storage/examples/hashes_example.ccAnalysis chain
LGTM! But verify the method usage in the codebase.
The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toExpireare updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Expire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Expire'Length of output: 53310
src/pika_cache.cc (1)
129-132: Update the parameter name in the following files to ensure consistency:
src/cache/src/cache.cc: Changettltomillisecondsin theExpireatmethod.include/pika_cache.h: Changettltomillisecondsin theExpireatmethod declaration.Analysis chain
LGTM! But verify the function usage in the codebase.
The parameter name change from
ttltomillisecondsimproves clarity and aligns with the PR objectives.However, ensure that all function calls to
Expireatmatch the new parameter name.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Expireat` match the new parameter name. # Test: Search for the function usage. Expect: Only occurrences of the new parameter name. rg --type cpp -A 5 $'Expireat'Length of output: 14462
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- include/pika_kv.h (4 hunks)
- src/pika_cache.cc (1 hunks)
- src/pika_kv.cc (14 hunks)
- src/storage/include/storage/storage.h (3 hunks)
- src/storage/src/base_value_format.h (4 hunks)
- src/storage/src/redis.h (3 hunks)
- src/storage/src/redis_hashes.cc (14 hunks)
- src/storage/src/redis_lists.cc (7 hunks)
- src/storage/src/redis_sets.cc (13 hunks)
- src/storage/src/redis_strings.cc (9 hunks)
- src/storage/src/redis_zsets.cc (15 hunks)
- src/storage/src/storage.cc (8 hunks)
Files skipped from review due to trivial changes (2)
- include/pika_kv.h
- src/storage/src/redis_zsets.cc
Files skipped from review as they are similar to previous changes (6)
- src/pika_kv.cc
- src/storage/src/base_value_format.h
- src/storage/src/redis_hashes.cc
- src/storage/src/redis_sets.cc
- src/storage/src/redis_strings.cc
- src/storage/src/storage.cc
Additional comments not posted (20)
src/storage/src/redis.h (11)
120-120: LGTM! But verify the method usage in the codebase.The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toStringsExpireare updated accordingly.Verification successful
Verified: Method usage is consistent with the new signature.
The
StringsExpiremethod has been updated to usettl_millsec, and all instances in the codebase reflect this change correctly.
src/storage/src/redis_strings.cc: Method definition and usage.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 cpp -A 5 $'StringsExpire'Length of output: 3017
121-121: LGTM! But verify the method usage in the codebase.The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toHashesExpireare updated accordingly.Verification successful
The method usage in the codebase is consistent with the new signature.
The change to use
ttl_millsecinstead ofttlhas been applied correctly in all instances ofHashesExpirein the codebase.
src/storage/src/redis_strings.ccsrc/storage/src/redis_hashes.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `HashesExpire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'HashesExpire'Length of output: 3208
122-122: LGTM! But verify the method usage in the codebase.The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toListsExpireare updated accordingly.Verification successful
Verification Complete: All usages of
ListsExpirehave been updated to usettl_millsec.The method signature change to use
ttl_millsecinstead ofttlis consistent across the codebase.
src/storage/src/redis_strings.ccsrc/storage/src/redis_lists.ccsrc/storage/src/redis.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ListsExpire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ListsExpire'Length of output: 3035
124-124: LGTM! But verify the method usage in the codebase.The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toSetsExpireare updated accordingly.Verification successful
Verified: The method usage is consistent with the updated signature.
The change to use
ttl_millsecinstead ofttlhas been correctly applied to all instances ofSetsExpirein the codebase.
src/storage/src/redis_strings.ccsrc/storage/src/redis_sets.ccsrc/storage/src/redis.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetsExpire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SetsExpire'Length of output: 3097
123-123: LGTM! But verify the method usage in the codebase.The change to use
ttl_millsecinstead ofttlis clear and improves the API's clarity. Ensure that all calls toZsetsExpireare updated accordingly.Verification successful
LGTM! The method usage is consistent with the new signature.
The change to use
ttl_millsecinstead ofttlhas been properly propagated throughout the codebase.
src/storage/src/redis_strings.cc: The methodZsetsExpireis called with the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsExpire` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ZsetsExpire'Length of output: 3088
133-133: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toStringsExpireatare updated accordingly.
134-134: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toHashesExpireatare updated accordingly.Verification successful
Verified: The method
HashesExpireathas been updated to use the new signature consistently across the codebase.
src/storage/src/redis_strings.cc: Calls toHashesExpireatusetimestamp_millsec.src/storage/src/redis_hashes.cc: Implementation ofHashesExpireatusestimestamp_millsec.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `HashesExpireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'HashesExpireat'Length of output: 1655
136-136: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toSetsExpireatare updated accordingly.Verification successful
Verification successful.
The method
SetsExpireathas been correctly updated to usetimestamp_millsecinstead oftimestampacross the codebase. The new signature is applied consistently in both the method definition and its usage.
src/storage/src/redis.hsrc/storage/src/redis_sets.ccsrc/storage/src/redis_strings.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetsExpireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SetsExpireat'Length of output: 1602
189-189: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toExpireatare updated accordingly.
135-135: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toListsExpireatare updated accordingly.Verification successful
Verification Successful: Method usage is consistent.
The
ListsExpireatmethod has been updated to usetimestamp_millsecinstead oftimestamp, and all occurrences in the codebase reflect this change. No instances of the old signature were found.
src/storage/src/redis_strings.ccsrc/storage/src/redis_lists.ccsrc/storage/src/redis.hScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ListsExpireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ListsExpireat'Length of output: 1562
Script:
#!/bin/bash # Description: Verify no old function calls to `ListsExpireat` with the previous signature exist. # Test: Search for the old function usage. Expect: No occurrences of the old signature. rg --type cpp -A 5 $'ListsExpireat(.*timestamp'Length of output: 138
Script:
#!/bin/bash # Description: Verify no old function calls to `ListsExpireat` with the previous signature exist. # Test: Search for the old function usage. Expect: No occurrences of the old signature. rg --type cpp -A 5 $'ListsExpireat\(.*timestamp'Length of output: 1577
137-137: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toZsetsExpireatare updated accordingly.Verification successful
Verification Complete: No issues found.
The method
ZsetsExpireathas been updated to usetimestamp_millsecconsistently throughout the codebase. No instances of the old method signature were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsExpireat` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ZsetsExpireat'Length of output: 1577
Script:
#!/bin/bash # Description: Verify there are no remaining instances of the old method signature `ZsetsExpireat` with `timestamp`. # Test: Search for the old function signature. Expect: No occurrences of the old signature. rg --type cpp -A 5 $'ZsetsExpireat(const Slice& key, int64_t timestamp'Length of output: 186
Script:
#!/bin/bash # Description: Verify there are no remaining instances of the old method signature `ZsetsExpireat` with `timestamp`. # Test: Search for the old function signature. Expect: No occurrences of the old signature. rg --type cpp -A 5 'ZsetsExpireat\(const Slice& key, int64_t timestamp'Length of output: 1093
src/storage/include/storage/storage.h (2)
1035-1035: LGTM! But verify the method usage in the codebase.The addition of the
PTTLmethod is a valuable enhancement for retrieving TTL in milliseconds. Ensure that the method is used correctly in the codebase.Verification successful
Verify the usage of the
PTTLmethod inpika_kv.cc.The
PTTLmethod is defined and implemented correctly. However, to ensure its proper usage, review the context around its usage inpika_kv.cc.
The usage of the
PTTLmethod is correct.The
PTTLmethod is correctly used inpika_kv.ccto retrieve the TTL of a key and handle potential internal errors appropriately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `PTTL` method in the codebase. # Test: Search for the method usage. Expect: Correct usage of the new method. rg --type cpp -A 5 $'PTTL'Length of output: 2142
Script:
#!/bin/bash # Description: Verify the context around the usage of the `PTTL` method in `pika_kv.cc`. # Test: Search for the method usage in `pika_kv.cc`. Expect: Correct usage of the new method. rg --type cpp -A 10 $'PTTL' src/pika_kv.ccLength of output: 340
1013-1013: LGTM! But verify the method usage in the codebase.The change to use
timestamp_millsecinstead oftimestampis clear and improves the API's clarity. Ensure that all calls toExpireatare updated accordingly.Verification successful
Verify remaining instances of the old
Expireatsignature.The output indicates that most instances of
Expireathave been updated to usetimestamp_millsec. However, a few instances might still be using the old signature. Please ensure that all occurrences have been updated.
All instances of
Expireathave been updated to the new signature.The search did not find any remaining instances of the old
Expireatsignature, confirming that all occurrences have been updated to usetimestamp_millsec.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 cpp -A 5 $'Expireat'Length of output: 14462
Script:
#!/bin/bash # Description: Verify if there are any remaining instances of the old `Expireat` signature. # Test: Search for the old function signature. Expect: No occurrences of the old signature. rg --type cpp -A 5 $'Expireat(std::string& key, int64_t ttl)'Length of output: 62
src/storage/src/redis_lists.cc (7)
33-33: LGTM!The change to retrieve the current time in milliseconds using
pstd::NowMillis()is appropriate and improves clarity.
505-505: LGTM!The change to retrieve the current time in milliseconds using
pstd::NowMillis()is appropriate and improves clarity.
1098-1098: LGTM!The change to use
ttl_millsecinstead ofttlaligns with the PR objectives and ensures TTL is handled in milliseconds.
1127-1128: LGTM!The method correctly updates the TTL in milliseconds.
1175-1175: LGTM!The change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives and ensures expiration time is handled in milliseconds.
1203-1204: LGTM!The method correctly updates the expiration time in milliseconds.
1288-1288: LGTM!The change to retrieve the current time in milliseconds using
pstd::NowMillis()is appropriate and improves clarity.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/tests/strings_test.cc (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/storage/tests/strings_test.cc
| @@ -643,10 +643,9 @@ class PexpireatCmd : public Cmd { | |||
|
|
|||
| private: | |||
| std::string key_; | |||
| int64_t time_stamp_ms_ = 0; | |||
There was a problem hiding this comment.
setex, psetex这两个命令也需要处理下。建议按照pika_command.h里的命令逐个梳理下，一个是调用的storage接口传的时间单位是否一致，另一个命令写binlog的时候传的ttl单位是否一致。
There was a problem hiding this comment.
总结了下，有如下这些命令会影响，都处理了。其中有一些命令的 binlog 或是 cache 会有精度损失，这里新建了一个 issue，另做处理：#2847
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- include/pika_cache.h (1 hunks)
- include/pika_kv.h (10 hunks)
- src/pika_cache.cc (1 hunks)
- src/pika_cache_load_thread.cc (1 hunks)
- src/pika_kv.cc (32 hunks)
- src/storage/include/storage/storage.h (15 hunks)
- src/storage/src/base_value_format.h (4 hunks)
- src/storage/src/redis.h (7 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_strings.cc (22 hunks)
- src/storage/src/redis_zsets.cc (17 hunks)
- src/storage/src/storage.cc (16 hunks)
- src/storage/tests/lists_filter_test.cc (2 hunks)
- src/storage/tests/strings_filter_test.cc (1 hunks)
Files skipped from review due to trivial changes (1)
- src/storage/src/redis_zsets.cc
Files skipped from review as they are similar to previous changes (9)
- include/pika_kv.h
- src/pika_cache.cc
- src/storage/include/storage/storage.h
- src/storage/src/base_value_format.h
- src/storage/src/redis_hashes.cc
- src/storage/src/redis_lists.cc
- src/storage/src/redis_sets.cc
- src/storage/src/redis_strings.cc
- src/storage/src/storage.cc
Additional comments not posted (29)
src/storage/tests/strings_filter_test.cc (1)
24-24: LGTM!The method
SetRelativeTimeByMillseccorrectly reflects the shift to handling TTL in milliseconds.src/pika_cache_load_thread.cc (1)
118-124: LGTM!The method
LoadSetcorrectly handles TTL in milliseconds and converts it to seconds when writing to the cache.src/storage/tests/lists_filter_test.cc (2)
105-105: LGTM!The method
SetRelativeTimeByMillseccorrectly reflects the shift to handling TTL in milliseconds.
122-122: LGTM!The method
SetRelativeTimeByMillseccorrectly reflects the shift to handling TTL in milliseconds.include/pika_cache.h (1)
70-70: Clarification of parameter name.The parameter name change from
ttltottl_secimproves clarity by explicitly indicating that the parameter represents a time value in seconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.src/storage/src/redis.h (13)
120-124: Clarification of parameter names to milliseconds.The parameter name changes to
ttl_millsecimprove clarity by explicitly indicating that the parameters represent time values in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
133-137: Clarification of parameter names to milliseconds for expiration at a specific timestamp.The parameter name changes to
timestamp_millsecimprove clarity by explicitly indicating that the parameters represent time values in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
145-149: Clarification of parameter names to milliseconds for TTL methods.The parameter name changes to
ttl_millsecimprove clarity by explicitly indicating that the parameters represent time values in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
159-160: Clarification of parameter names to milliseconds for GetWithTTL methods.The parameter name changes to
ttl_millsecimprove clarity by explicitly indicating that the parameters represent time values in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
164-164: Clarification of parameter names to milliseconds for GetrangeWithValue method.The parameter name change to
ttl_millsecimproves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
172-176: Clarification of parameter names to milliseconds for Set methods.The parameter name changes to
ttl_millsecimprove clarity by explicitly indicating that the parameters represent time values in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
184-184: Clarification of parameter name to milliseconds for PKSetexAt method.The parameter name change to
time_stamp_millsec_improves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
188-189: Clarification of parameter names to milliseconds for Expire methods.The parameter name changes to
ttl_millsecandtimestamp_millsecimprove clarity by explicitly indicating that the parameters represent time values in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
191-191: Clarification of parameter name to milliseconds for TTL method.The parameter name change to
ttl_millsecimproves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
201-201: Clarification of parameter name to milliseconds for HGetallWithTTL method.The parameter name change to
ttl_millsecimproves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
258-258: Clarification of parameter name to milliseconds for SMembersWithTTL method.The parameter name change to
ttl_millsecimproves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
279-279: Clarification of parameter name to milliseconds for LRangeWithTTL method.The parameter name change to
ttl_millsecimproves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.
294-294: Clarification of parameter name to milliseconds for ZRangeWithTTL method.The parameter name change to
ttl_millsecimproves clarity by explicitly indicating that the parameter represents a time value in milliseconds. This enhances code readability and reduces potential confusion regarding the unit of measurement.src/pika_kv.cc (11)
25-25: Initializettl_millsecto 0.The initialization of
ttl_millsecto 0 is correct and ensures that TTL is handled in milliseconds.
54-55: Convert TTL from seconds to milliseconds.The conversion of TTL from seconds to milliseconds using
ttl_millsec *= 1000is correct and ensures consistency in TTL handling.
70-79: Update storage calls to usettl_millsec.The updates to the storage calls to use
ttl_millsecensure that TTL is handled in milliseconds.
114-114: Update cache calls to usettl_millsec / 1000.The updates to the cache calls to use
ttl_millsec / 1000ensure that TTL is handled correctly in seconds.
161-161: Update storage call to useGetWithTTL.The update to the storage call to use
GetWithTTLensures that TTL is retrieved in milliseconds.
191-191: Update cache call to usettl_millsec_ / 1000.The update to the cache call to use
ttl_millsec_ / 1000ensures that TTL is handled correctly in seconds.
496-496: AddAppendCmd::DoThroughDBmethod.The addition of the
AppendCmd::DoThroughDBmethod follows the pattern used in other command methods.
534-534: Update storage call to useMGetWithTTL.The update to the storage call to use
MGetWithTTLensures that TTL is retrieved in milliseconds.
606-607: Update cache call to usettl_millsec / 1000.The update to the cache call to use
ttl_millsec / 1000ensures that TTL is handled correctly in seconds.
749-749: Update storage call to usettl_sec_ * 1000.The update to the storage call to use
ttl_sec_ * 1000ensures that TTL is handled correctly in milliseconds.
766-766: Update cache call to usettl_sec_.The update to the cache call to use
ttl_sec_ensures that TTL is handled correctly in seconds.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- .github/workflows/pika.yml (2 hunks)
- src/pika_admin.cc (2 hunks)
- src/pika_cache_load_thread.cc (1 hunks)
- src/pika_conf.cc (2 hunks)
- src/pika_server.cc (5 hunks)
- src/storage/include/storage/storage.h (15 hunks)
- src/storage/src/redis.h (7 hunks)
- src/storage/src/redis_hashes.cc (16 hunks)
- src/storage/src/redis_strings.cc (22 hunks)
- src/storage/src/storage.cc (16 hunks)
- src/storage/tests/keys_test.cc (1 hunks)
Files skipped from review due to trivial changes (4)
- .github/workflows/pika.yml
- src/pika_admin.cc
- src/pika_conf.cc
- src/pika_server.cc
Files skipped from review as they are similar to previous changes (6)
- src/pika_cache_load_thread.cc
- src/storage/include/storage/storage.h
- src/storage/src/redis_hashes.cc
- src/storage/src/redis_strings.cc
- src/storage/src/storage.cc
- src/storage/tests/keys_test.cc
Additional comments not posted (29)
src/storage/src/redis.h (29)
120-120: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
121-121: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
122-122: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
123-123: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
124-124: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
133-133: Update to milliseconds for timestamp values approved.The method signature change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives.
134-134: Update to milliseconds for timestamp values approved.The method signature change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives.
135-135: Update to milliseconds for timestamp values approved.The method signature change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives.
136-136: Update to milliseconds for timestamp values approved.The method signature change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives.
137-137: Update to milliseconds for timestamp values approved.The method signature change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives.
145-145: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
146-146: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
147-147: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
148-148: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
149-149: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
159-159: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
160-160: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
164-164: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
172-172: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
174-174: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
175-175: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
176-176: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
188-188: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
189-189: Update to milliseconds for timestamp values approved.The method signature change to use
timestamp_millsecinstead oftimestampaligns with the PR objectives.
191-191: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
201-201: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
258-258: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
279-279: Update to milliseconds for TTL values approved.The method signature change to use
ttl_millsecinstead ofttlaligns with the PR objectives.
294-294: **Update to
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
src/storage/src/redis.h (2)
188-188: Update allExpiremethod calls to use millisecond-based TTL values.The parameter name change from
ttltottl_millsecindicates that the TTL should be provided in milliseconds. However, there are still instances where theExpiremethod is called with second-based TTL values. Please update allExpiremethod calls to ensure the TTL values are in milliseconds.
- src/pika_kv.cc: Lines 188-188
- src/storage/tests/lists_test.cc: Lines 188-188
- src/storage/tests/keys_test.cc: Lines 188-188
- src/storage/tests/strings_test.cc: Lines 188-188
- src/storage/tests/sets_test.cc: Lines 188-188
- src/storage/tests/hashes_test.cc: Lines 188-188
Analysis chain
LGTM! Verify the usage of
Expire.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
Expirepass the TTL value in milliseconds.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Expire` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Expire'Length of output: 53405
189-189: Ensure all calls toExpireatpass the timestamp in milliseconds.The search results indicate that there is an instance where
time_stamp_sec_is used directly without conversion to milliseconds. This could lead to incorrect expiration times.
- File:
src/pika_kv.cc
- Line:
db_->cache()->Expireat(key_, time_stamp_sec_);- Issue:
time_stamp_sec_should be converted to milliseconds before being passed toExpireat.Please update this instance to ensure the timestamp is in milliseconds.
Analysis chain
LGTM! Verify the usage of
Expireat.The parameter name change from
timestamptotimestamp_millsecis consistent with the objective of handling timestamps in milliseconds.Ensure that all calls to
Expireatpass the timestamp value in milliseconds.
Ensure all calls to
Expireatpass the timestamp in milliseconds.The search results indicate that there are instances where
Expireatis called withtime_stamp_sec_, which might be in seconds. We need to verify if these instances are correctly converted to milliseconds before being passed toExpireat.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Expireat` pass the timestamp value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'Expireat'Length of output: 14537
Script:
#!/bin/bash # Description: Verify if `time_stamp_sec_` is converted to milliseconds before being passed to `Expireat`. # Search for instances where `time_stamp_sec_` is used in `Expireat` calls. rg --type cpp 'Expireat\(.*time_stamp_sec_' -A 5Length of output: 583
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/src/redis.h (7 hunks)
Additional comments not posted (15)
src/storage/src/redis.h (15)
122-122: LGTM! Verify the usage ofListsExpire.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
ListsExpirepass the TTL value in milliseconds.
124-124: LGTM! Verify the usage ofSetsExpire.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
SetsExpirepass the TTL value in milliseconds.Verification successful
Verification successful.
The parameter name change from
ttltottl_millsechas been consistently applied across the codebase, and all calls toSetsExpirepass the TTL value in milliseconds.
src/storage/src/redis_strings.cc: Verified usage ofSetsExpirewithttl_millsec.src/storage/src/redis_sets.cc: Verified method definition withttl_millsec.src/storage/src/redis.h: Verified method declaration withttl_millsec.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetsExpire` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SetsExpire'Length of output: 3097
135-135: LGTM! Verify the usage ofListsExpireat.The parameter name change from
timestamptotimestamp_millsecis consistent with the objective of handling timestamps in milliseconds.Ensure that all calls to
ListsExpireatpass the timestamp value in milliseconds.
148-148: LGTM! Verify the usage ofZsetsTTL.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
ZsetsTTLpass the TTL value in milliseconds.Verification successful
LGTM! Verify the usage of
ZsetsTTL.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds. The search results confirm that the function is being called with the new parameter name.
src/storage/src/redis_strings.cc: Verified thatZsetsTTLis called withttl_millsec.src/storage/src/redis_zsets.cc: Verified that the function definition matches the new signature.Ensure that all calls to
ZsetsTTLpass the TTL value in milliseconds.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsTTL` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ZsetsTTL'Length of output: 1505
123-123: LGTM! Verify the usage ofZsetsExpire.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
ZsetsExpirepass the TTL value in milliseconds.Verification successful
Verified: All calls to
ZsetsExpirepass the TTL value in milliseconds.The parameter name change from
ttltottl_millsecis consistent and correctly implemented across the codebase.
src/storage/src/redis_strings.cc:ZsetsExpire(key, ttl_millsec, std::move(meta_value))src/storage/src/redis.h: Method declaration withttl_millsecsrc/storage/src/redis_zsets.cc: Method definition withttl_millsecScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsExpire` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ZsetsExpire'Length of output: 3088
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsExpire` pass the TTL value in milliseconds. # Test: Search for all function calls to `ZsetsExpire`. Expect: Only occurrences of the new signature. rg --type cpp 'ZsetsExpire\('Length of output: 387
120-120: LGTM! Verify the usage ofStringsExpire.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
StringsExpirepass the TTL value in milliseconds.Verification successful
Verification complete.
The parameter name change from
ttltottl_millsecis consistent and logical. All calls toStringsExpirepass the TTL value in milliseconds.
src/storage/src/redis.h: Method definition.src/storage/src/redis_strings.cc: Method implementation and usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StringsExpire` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'StringsExpire'Length of output: 3017
Script:
#!/bin/bash # Description: Verify all function calls to `StringsExpire` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp 'StringsExpire\('Length of output: 397
147-147: LGTM! Verify the usage ofListsTTL.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
ListsTTLpass the TTL value in milliseconds.Verification successful
Verify all instances of
ListsTTLusettl_millsec.We need to ensure that all instances of
ListsTTLacross the codebase use the new parameter namettl_millsec.
Verification successful.
All instances of
ListsTTLin the codebase use the new parameter namettl_millsec, confirming the change has been consistently applied.
src/storage/src/redis_strings.ccsrc/storage/src/redis.hsrc/storage/src/redis_lists.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ListsTTL` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ListsTTL'Length of output: 1437
Script:
#!/bin/bash # Description: Verify all instances of `ListsTTL` use `ttl_millsec`. # Test: Search for all occurrences of `ListsTTL` and check the parameter usage. rg --type cpp 'ListsTTL'Length of output: 373
134-134: LGTM! Verify the usage ofHashesExpireat.The parameter name change from
timestamptotimestamp_millsecis consistent with the objective of handling timestamps in milliseconds.Ensure that all calls to
HashesExpireatpass the timestamp value in milliseconds.
121-121: LGTM! Verify the usage ofHashesExpire.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
HashesExpirepass the TTL value in milliseconds.Verification successful
Verify the usage of
HashesExpireacross the codebase.Ensure that all calls to
HashesExpirepass the TTL value in milliseconds.
Verification successful.
The parameter name change from
ttltottl_millsecis consistent across the codebase, and all calls toHashesExpirepass the TTL value in milliseconds.
src/storage/src/redis_strings.ccsrc/storage/src/redis_hashes.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `HashesExpire` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'HashesExpire'Length of output: 3208
Script:
#!/bin/bash # Comprehensive search for all usages of `HashesExpire` to verify consistency in parameter usage. # Search for all occurrences of `HashesExpire` in the codebase. rg --type cpp -A 5 'HashesExpire'Length of output: 3208
136-136: LGTM! Verify the usage ofSetsExpireat.The parameter name change from
timestamptotimestamp_millsecis consistent with the objective of handling timestamps in milliseconds.Ensure that all calls to
SetsExpireatpass the timestamp value in milliseconds.Verification successful
Verification Successful: Consistent Usage of
timestamp_millsecinSetsExpireatThe parameter name change from
timestamptotimestamp_millsecis consistently applied across the codebase, ensuring that all calls toSetsExpireatpass the timestamp value in milliseconds.
src/storage/src/redis.hsrc/storage/src/redis_sets.ccsrc/storage/src/redis_strings.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetsExpireat` pass the timestamp value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SetsExpireat'Length of output: 1602
Script:
#!/bin/bash # Description: Verify all instances of `SetsExpireat` in the codebase to ensure consistent usage of the `timestamp_millsec` parameter. # Search for all instances of `SetsExpireat` in the codebase. rg --type cpp 'SetsExpireat'Length of output: 413
149-149: LGTM! Verify the usage ofSetsTTL.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
SetsTTLpass the TTL value in milliseconds.Verification successful
Verification Successful!
All calls to
SetsTTLpass the TTL value in milliseconds as expected. The parameter name change fromttltottl_millsecis consistent and correctly implemented.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetsTTL` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'SetsTTL'Length of output: 1554
Script:
#!/bin/bash # Description: Verify all function calls to `SetsTTL` pass the TTL value in milliseconds. # Test: Search for all instances of `SetsTTL` in the codebase. rg --type cpp 'SetsTTL'Length of output: 377
137-137: LGTM! Verify the usage ofZsetsExpireat.The parameter name change from
timestamptotimestamp_millsecis consistent with the objective of handling timestamps in milliseconds.Ensure that all calls to
ZsetsExpireatpass the timestamp value in milliseconds.Verification successful
Verified: The parameter name change from
timestamptotimestamp_millsechas been consistently applied across the codebase.
src/storage/src/redis.hsrc/storage/src/redis_strings.ccsrc/storage/src/redis_zsets.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsExpireat` pass the timestamp value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'ZsetsExpireat'Length of output: 1577
Script:
#!/bin/bash # Description: Verify all function calls to `ZsetsExpireat` pass the timestamp value in milliseconds. # Test: Search for all function calls to `ZsetsExpireat` and ensure they use `timestamp_millsec`. rg --type cpp 'ZsetsExpireat' -A 2Length of output: 879
146-146: LGTM! Verify the usage ofHashesTTL.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
HashesTTLpass the TTL value in milliseconds.Verification successful
Ensure TTL values are passed in milliseconds.
The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds. The search results show the new signature being used in relevant files.
- Files to check:
src/storage/src/redis_strings.ccsrc/storage/src/redis_hashes.ccVerify that all calls to
HashesTTL,ListsTTL,ZsetsTTL, andSetsTTLpass the TTL value in milliseconds.
Verified: TTL values are passed in milliseconds.
The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds. The search results confirm that all calls toHashesTTL,ListsTTL,ZsetsTTL, andSetsTTLpass the TTL value in milliseconds.
- Files verified:
src/storage/src/redis_strings.ccsrc/storage/src/redis_hashes.ccScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `HashesTTL` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'HashesTTL'Length of output: 1524
Script:
#!/bin/bash # Verify the context around HashesTTL, ListsTTL, ZsetsTTL, and SetsTTL to ensure TTL values are passed in milliseconds. # Check the context in redis_strings.cc rg --type cpp -A 10 'HashesTTL|ListsTTL|ZsetsTTL|SetsTTL' src/storage/src/redis_strings.cc # Check the context in redis_hashes.cc rg --type cpp -A 10 'HashesTTL' src/storage/src/redis_hashes.ccLength of output: 1314
145-145: LGTM! Verify the usage ofStringsTTL.The parameter name change from
ttltottl_millsecis consistent with the objective of handling TTL in milliseconds.Ensure that all calls to
StringsTTLpass the TTL value in milliseconds.Verification successful
Verified: The parameter name change from
ttltottl_millsecis consistent and correctly implemented.The usage of
ttl_millsecthroughout the codebase confirms that TTL values are being handled in milliseconds as intended.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StringsTTL` pass the TTL value in milliseconds. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type cpp -A 5 $'StringsTTL'Length of output: 1478
Script:
#!/bin/bash # Description: Verify all function calls to `StringsTTL` pass the TTL value in milliseconds. # Test: Search for all calls to `StringsTTL` and check the context to ensure TTL value is in milliseconds. rg --type cpp 'StringsTTL' -A 3 -B 3Length of output: 1724
Script:
#!/bin/bash # Description: Verify that the variables passed to `StringsTTL` represent TTL in milliseconds. # Test: Search for the definitions and usages of the variables passed to `StringsTTL`. rg --type cpp 'ttl_millsec' -A 3 -B 3Length of output: 65757
133-133: LGTM! Verify the usage ofStringsExpireat.The parameter name change from
timestamptotimestamp_millsecis consistent with the objective of handling timestamps in milliseconds.Ensure that all calls to
StringsExpireatpass the timestamp value in milliseconds.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/tests/strings_test.cc (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/storage/tests/strings_test.cc
…undation#2822) * change storage ttl time from seconds to milliseconds --------- Co-authored-by: liuyuecai <[email protected]> test space
…clear space in CI after compile (#2853) * fix:remove dependence in CI * Revert "fix: change storage ttl time from seconds to milliseconds (#2822)" --------- Co-authored-by: cjh <[email protected]> Co-authored-by: chejinge <[email protected]>
* fix: change storage ttl time from seconds to milliseconds (#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]>
…undation#2822) * change storage ttl time from seconds to milliseconds --------- Co-authored-by: liuyuecai <[email protected]>
…clear space in CI after compile (OpenAtomFoundation#2853) * fix:remove dependence in CI * Revert "fix: change storage ttl time from seconds to milliseconds (OpenAtomFoundation#2822)" --------- Co-authored-by: cjh <[email protected]> Co-authored-by: chejinge <[email protected]>
…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]>
fix #2821
1、将 floyd 存储的 date 字段的值从秒改为了 millseconds；
2、将 version 字段的值从秒改为了 micro-seconds；
3、floyd 中的 ctime 和 etime 字段，最高位的 bit，如果为1，表示毫秒单位，如果为0，表示秒级单位。因为之前存储的数据都是秒，这样能兼容历史版本的数据
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Tests