fix:SETEX replication does not work#3122
fix:SETEX replication does not work#3122YuCai18 wants to merge 1 commit intoOpenAtomFoundation:unstablefrom
Conversation
|
""" WalkthroughThe changes improve expiration timestamp handling in key expiration and set-with-expiration commands by explicitly using 64-bit integer types and converting between milliseconds and seconds as needed. This ensures consistent, overflow-safe timestamp calculations and Redis protocol output. Changes
Estimated code review effort2 (~20 minutes) Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| RedisAppendLenUint64(content, pksetexat_cmd.size(), "$"); | ||
| RedisAppendContent(content, pksetexat_cmd); | ||
| // to setex cmd | ||
| std::string setex_cmd("setex"); |
There was a problem hiding this comment.
这里不能直接这样修改,因为setex设置过期时间的方式是将当前机器的时间加上+TTL, 而 pksetexat 命令设置过期时间的方式是设置绝对的过期时间戳,由于每个机器上机器时钟可能不一致,所以理应是采用第二种方式
| RedisAppendLenUint64(content, pksetexat_cmd.size(), "$"); | ||
| RedisAppendContent(content, pksetexat_cmd); | ||
| // to psetex cmd | ||
| std::string psetex_cmd("psetex"); |
There was a problem hiding this comment.
为什么psetex这里也需要改动,我记得issue提到的是setex命令有问题
| // time_stamp | ||
| char buf[100]; | ||
| auto time_stamp = pstd::NowMillis() + ttl_millsec; | ||
| auto time_stamp = ttl_millsec; |
There was a problem hiding this comment.
这里的pstd::NowMillis()会不会有问题,这个返回值我记得是毫秒的单位
There was a problem hiding this comment.
这里没有问题,因为查看pika_kv.h代码可以发现pksetexat 期望的 第 2 个参数的秒级,它在里面会/1000,所以这里要用毫秒。
|
done @Mixficsol |
done @Mixficsol |
修复了pika4.0中setex在主从上不同步的bug #3117
修改的代码部分
1.命令名称错误:
修改了SetexCmd::ToRedisProtocol()和PsetexCmd::ToRedisProtocol()方法。原代码中,SetexCmd::ToRedisProtocol()方法生成的是pksetexat命令,而不是标准的setex命令。同样,PsetexCmd::ToRedisProtocol()也生成了错误的命令名称。在主从复制过程中,主节点会将命令转换为二进制日志(binlog)发送给从节点,所以从节点无法正确识别和执行这些非标准命令。
2.过期时间计算错误:
原代码中计算过期时间使用了time(nullptr) + ttl_sec_(对于SETEX)和pstd::NowMillis() + ttl_millsec(对于PSETEX)。这意味着主节点计算的是"当前时间+过期秒数"作为绝对过期时间点。当从节点接收到这个命令时,它会直接使用这个绝对时间,而不是重新计算。由于主从之间可能存在时间差或者命令传输延迟,这会导致从节点上的实际过期时间与主节点不一致。
修改后的运行截图
Summary by CodeRabbit