-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs:4.0.1 changelog #2902
docs:4.0.1 changelog #2902
Conversation
WalkthroughThe changes in version 4.0.1 of the Pika service introduce new features, improvements, and bug fixes aimed at enhancing functionality and performance. Key updates include a switch for RTC functionality allowing direct database reads on cache misses, optimized access cache, and a configurable log deletion task. Improvements focus on data cleaning, Redis compatibility, and enhanced command handling. Bug fixes address cache inaccuracies, command exceptions, and replication issues, ensuring better consistency and reliability across the system. Changes
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (3)
Files skipped from review as they are similar to previous changes (2)
Additional context usedMarkdownlint
Additional comments not posted (3)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range and nitpick comments (5)
CHANGELOG.MD (5)
1-1
: Standardize version notation to lowercase 'v'The version notation on line 1 is 'V4.0.1' with an uppercase 'V'. For consistency with other versions in the changelog (e.g., 'v3.5.5'), please change it to lowercase 'v'.
11-11
: Standardize section heading to 'Improvements'On line 11, the section heading is '## Improvement'. To maintain consistency with standard conventions and other sections in the changelog, please rename it to '## Improvements'.
27-27
: Standardize section heading to 'Bugfixes'The section heading on line 27 is '## Bugfix'. For consistency with other sections and standard terminology, please update it to '## Bugfixes'.
Line range hint
65-275
: Remove previous versions' changelog entries from v4.0.1Starting from line 65, the changelog includes entries for previous versions (e.g., v3.5.5, v3.5.4, etc.). To keep the changelog for v4.0.1 focused and concise, please remove these entries or move them to the appropriate version sections.
15-15
: Verify the possessive form in 'Floyd’s storeddata
field'On line 15, it reads 'Changed the value in Floyd’s stored
data
field to milliseconds...'. Please verify if 'Floyd’s' is correctly possessive or if it should be 'Floyd storeddata
field' without the apostrophe.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.MD (1 hunks)
- CHANGELOG_CN.MD (1 hunks)
Additional context used
Markdownlint
CHANGELOG_CN.MD
65-65: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
1ffe39d
to
b9b73e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range and nitpick comments (5)
tests/integration/zset_test.go (1)
1483-1483
: Consider asserting on the exact expected result for a stricter test.The assertion has been changed from checking for an exact match with
[]string{"m3"}
to checking that the result is not empty. While this broadens the acceptance criteria, it could potentially allow false positives if the result contains unexpected values.Consider still asserting on the exact expected result for a stricter and more precise test.
CHANGELOG.MD (3)
7-8
: Looks good! Consider providing more details in the description.Optimizing the Pika access cache using the RTC model to improve read performance sounds great. The change itself looks fine. In the future, consider elaborating a bit more in the description on how the RTC model is used to optimize the cache, as it would help readers understand the change better without having to look at the PR.
13-14
: Please provide more details on the data cleaning function.A data cleaning function to help with upgrades sounds potentially useful. However, the current description is quite vague. Could you please elaborate on what kind of data cleaning is performed by this function and how exactly it is meant to be used during the upgrade process? Providing more context will help users better understand the purpose and scope of this feature.
23-24
: Looks good! Consider providing a bit more detail in the description.Modifying the replica node binlog consumption thread model to ensure proper binlog consumption order is important for data consistency. The change itself looks fine based on the PR. For the changelog entry, consider elaborating a bit more on what changes were made to the thread model to achieve the proper ordering, as it would help readers understand the gist of the change without having to look at the PR.
CHANGELOG_CN.MD (1)
14-14
: Rephrase "清洗数据" for clarityConsider changing "进行清洗数据" to "进行数据清洗" to improve readability.
Apply this diff to enhance the sentence:
- 添加数据清洗函数,方便用户在升级过程中进行清洗数据[#2888](https://github.com/OpenAtomFoundation/pika/pull/2888)@[QlQlqiqi](https://github.com/QlQlqiqi) + 添加数据清洗函数,方便用户在升级过程中进行数据清洗[#2888](https://github.com/OpenAtomFoundation/pika/pull/2888)@[QlQlqiqi](https://github.com/QlQlqiqi)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.MD (1 hunks)
- CHANGELOG_CN.MD (1 hunks)
- tests/integration/zset_test.go (1 hunks)
Additional context used
Markdownlint
CHANGELOG_CN.MD
63-63: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
Additional comments not posted (8)
CHANGELOG.MD (7)
5-6
: LGTM!The feature to bypass the cache and directly read from the DB in certain scenarios when using the RTC function seems like a good optimization. The description is clear.
9-10
: Looks good!Adding a scheduled task to automatically delete old logs with a configurable deletion frequency is a helpful feature for maintenance. The description is clear and no issues stand out.
15-16
: Looks good!Changing Floyd's stored
data
field to milliseconds for compatibility with Redis commands is a good change for consistency. The description is clear and the change itself looks fine.
17-18
: Looks good!The change to write individual
flushdb
entries to the binlog for each DB whenflushall
is called in multi-DB mode is a good one. It ensures proper consumption order on replica nodes for data consistency. The description explains the change and reasoning clearly.
19-20
: Looks good!Removing unnecessary logs to reduce excessive disk space usage that could impact data read/write performance is a good optimization. The motivation is clearly explained. Without knowing the specific logs that were removed, I don't have any other comments.
21-22
: Looks good!Changing the
incr
andappend
commands to usepksetexat
when transferring binlog in order to prevent incorrect operations that could cause data expiration issues and dirty data is a good move for data integrity. The description outlines the problem and solution well.
25-26
: Looks good!Adding the
open_rocksdb_statistics_tickers
configuration field to enable additional RocksDB metrics is a nice addition for observability. Defaulting it tono
and mentioning the performance cost when enabled is a good approach to make it opt-in and inform users of the tradeoff. The description is clear and informative.CHANGELOG_CN.MD (1)
63-63
: Duplicate issue: spacing inside emphasis markersThis issue has been previously noted. Please refer to the prior review comment to resolve it.
Tools
Markdownlint
63-63: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
- 修复 BlockCache 计算不准确的问题[#2797](https://github.com/OpenAtomFoundation/pika/pull/2797)@[bigdaronlee163](https://github.com/bigdaronlee163) | ||
|
||
- 添加标志位、时间戳和返回值机制,确保 Pika 在执行 flushdb 且处理异步删除旧目录时冲突时正确处理[#2790](https://github.com/OpenAtomFoundation/pika/pull/2790)@[cheniujh](https://github.com/cheniujh) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant "时" in the sentence
There is a redundant "时" in "冲突时正确处理". Please remove the extra character for clarity.
Apply this diff to correct the sentence:
- 添加标志位、时间戳和返回值机制,确保 Pika 在执行 flushdb 且处理异步删除旧目录时冲突时正确处理[#2790](https://github.com/OpenAtomFoundation/pika/pull/2790)@[cheniujh](https://github.com/cheniujh)
+ 添加标志位、时间戳和返回值机制,确保 Pika 在执行 flushdb 且处理异步删除旧目录冲突时正确处理[#2790](https://github.com/OpenAtomFoundation/pika/pull/2790)@[cheniujh](https://github.com/cheniujh)
Committable suggestion was skipped due to low confidence.
|
||
- 添加标志位、时间戳和返回值机制,确保 Pika 在执行 flushdb 且处理异步删除旧目录时冲突时正确处理[#2790](https://github.com/OpenAtomFoundation/pika/pull/2790)@[cheniujh](https://github.com/cheniujh) | ||
|
||
- 修复集群模式主备自动容灾过程中,老主节点降备过程中,因为 sent_offset 和 acked_offset 不相等,导致状态 Error 的情况[#2789](https://github.com/OpenAtomFoundation/pika/pull/2714)@[luky116](https://github.com/luky116) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure PR number matches the link
The PR number [#2789] does not match the link (which points to PR #2714). Please correct the PR number or update the link accordingly.
Apply this diff to correct the PR number:
- 修复集群模式主备自动容灾过程中,老主节点降备过程中,因为 sent_offset 和 acked_offset 不相等,导致状态 Error 的情况[#2789](https://github.com/OpenAtomFoundation/pika/pull/2714)@[luky116](https://github.com/luky116)
+ 修复集群模式主备自动容灾过程中,老主节点降备过程中,因为 sent_offset 和 acked_offset 不相等,导致状态 Error 的情况[#2714](https://github.com/OpenAtomFoundation/pika/pull/2714)@[luky116](https://github.com/luky116)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- 修复集群模式主备自动容灾过程中,老主节点降备过程中,因为 sent_offset 和 acked_offset 不相等,导致状态 Error 的情况[#2789](https://github.com/OpenAtomFoundation/pika/pull/2714)@[luky116](https://github.com/luky116) | |
- 修复集群模式主备自动容灾过程中,老主节点降备过程中,因为 sent_offset 和 acked_offset 不相等,导致状态 Error 的情况[#2714](https://github.com/OpenAtomFoundation/pika/pull/2714)@[luky116](https://github.com/luky116) |
CHANGELOG_CN.MD
Outdated
|
||
- incr、append 命令在传输 binlog 时,使用 pksetexat 命令,防止因为不正确的操作导致数据无法过期,出现脏数据[#2833](https://github.com/OpenAtomFoundation/pika/pull/2833)@[chejinge](https://github.com/chejinge) | ||
|
||
- 修改 Pika 从节点消费 binlog 的县城模型,保证 bingo 的消费顺序 [#2708](https://github.com/OpenAtomFoundation/pika/pull/2708)@[cheniujh](https://github.com/cheniujh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct typos: "县城模型" and "bingo"
The term "县城模型" should be "线程模型", and "bingo" should be "binlog".
Apply this diff to fix the typos:
- 修改 Pika 从节点消费 binlog 的县城模型,保证 bingo 的消费顺序 [#2708](https://github.com/OpenAtomFoundation/pika/pull/2708)@[cheniujh](https://github.com/cheniujh)
+ 修改 Pika 从节点消费 binlog 的线程模型,保证 binlog 的消费顺序 [#2708](https://github.com/OpenAtomFoundation/pika/pull/2708)@[cheniujh](https://github.com/cheniujh)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- 修改 Pika 从节点消费 binlog 的县城模型,保证 bingo 的消费顺序 [#2708](https://github.com/OpenAtomFoundation/pika/pull/2708)@[cheniujh](https://github.com/cheniujh) | |
- 修改 Pika 从节点消费 binlog 的线程模型,保证 binlog 的消费顺序 [#2708](https://github.com/OpenAtomFoundation/pika/pull/2708)@[cheniujh](https://github.com/cheniujh) |
CHANGELOG_CN.MD
Outdated
|
||
- 修复 Rpushx 命令执行时未更新 RedisCache 的问题,避免出现 DB、缓存不一致的问题[#2879](https://github.com/OpenAtomFoundation/pika/pull/2879)@[hahahashen](https://github.com/hahahashen) | ||
|
||
- 修复 kill client 命令杀连接流程不正确的[#2862](https://github.com/OpenAtomFoundation/pika/pull/2862)@[cheniujh](https://github.com/cheniujh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add "的问题" to complete the sentence
The description is incomplete. Adding "的问题" at the end clarifies the issue.
Apply this diff to complete the sentence:
- 修复 kill client 命令杀连接流程不正确的[#2862](https://github.com/OpenAtomFoundation/pika/pull/2862)@[cheniujh](https://github.com/cheniujh)
+ 修复 kill client 命令杀连接流程不正确的问题[#2862](https://github.com/OpenAtomFoundation/pika/pull/2862)@[cheniujh](https://github.com/cheniujh)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- 修复 kill client 命令杀连接流程不正确的[#2862](https://github.com/OpenAtomFoundation/pika/pull/2862)@[cheniujh](https://github.com/cheniujh) | |
- 修复 kill client 命令杀连接流程不正确的问题[#2862](https://github.com/OpenAtomFoundation/pika/pull/2862)@[cheniujh](https://github.com/cheniujh) |
@@ -1480,7 +1480,7 @@ var _ = Describe("Zset Commands", func() { | |||
|
|||
vals, err = client.ZRange(ctx, "zset1", 0, -1).Result() | |||
Expect(err).NotTo(HaveOccurred()) | |||
Expect(vals).To(Equal([]string{"m3"})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个为什么需要改
Co-authored-by: chejinge <chejinge@360.cn>
Co-authored-by: chejinge <chejinge@360.cn>
Co-authored-by: chejinge <chejinge@360.cn>
Summary by CodeRabbit
New Features
Improvements
flushall
command for better multi-DB logging and consumption order.Bug Fixes