Add consistency test for ClientInfo update failure in PushPull #1000
Conversation
WalkthroughThe changes involve updating the Changes
Possibly related PRs
Suggested labels
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- server/packs/packs.go (4 hunks)
- server/packs/packs_test.go (1 hunks)
- server/rpc/yorkie_server.go (4 hunks)
- test/bench/push_pull_bench_test.go (6 hunks)
Additional context used
GitHub Check: build
server/packs/packs.go
[failure] 42-42:
exported: exported var ErrCheckpointTest should have comment or be unexported (revive)
Additional comments not posted (7)
server/packs/packs.go (1)
Line range hint
77-135: Review the modifications in thePushPullfunction.The addition of the
cpTestboolean parameter is used to control the flow of execution for testing purposes. WhencpTestis true, the function returnsErrCheckpointTest, which simulates a failure scenario for consistency checks.Considerations:
- Ensure that this new testing pathway does not interfere with the normal operation of the
PushPullfunction.- Verify that all existing calls to
PushPullhave been appropriately updated to include the new parameter, to avoid breaking changes.Verification successful
All
PushPullfunction calls have been updated to include thecpTestparameter. The verification confirms that the new testing pathway does not interfere with the normal operation of thePushPullfunction, and all existing calls have been appropriately updated to include the new parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `PushPull` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'PushPull'Length of output: 68992
Script:
#!/bin/bash # Description: Verify all function calls to `PushPull` include the `cpTest` parameter. # Test: Search for the function usage and check for the presence of the `cpTest` parameter. rg --type go 'PushPull\(' -A 5 | grep -B 5 'cpTest'Length of output: 51
Script:
#!/bin/bash # Description: Search for all occurrences of `PushPull` and manually verify the presence of the `cpTest` parameter. # Test: Search for the function usage and display the surrounding lines for manual inspection. rg --type go 'PushPull\(' -A 5Length of output: 28314
server/packs/packs_test.go (1)
127-231: Approve the new test implementations inserver/packs/packs_test.go.The tests are well-structured and effectively simulate different scenarios to test the new
cpTestparameter in thePushPullfunction. These tests ensure that the function behaves as expected under both normal and simulated failure conditions.test/bench/push_pull_bench_test.go (1)
Line range hint
150-272: Approve the modifications in benchmark tests.The updates to include the
cpTestparameter with a value offalsein thePushPullfunction calls are correctly implemented. These changes ensure that the benchmark tests reflect the performance of the function under normal conditions, which is crucial for accurate performance evaluation.server/rpc/yorkie_server.go (4)
167-167: Verify the impact of the new boolean parameter inPushPull.The addition of the
falseparameter to thePushPullfunction call inAttachDocumentmethod is consistent with the PR's objective to introduce a consistency test. However, it's crucial to verify that this change aligns with the expected behavior of thePushPullfunction and does not introduce any unintended side effects.Verification successful
The addition of the boolean parameter in
PushPullis consistent and intentional.The boolean parameter added to the
PushPullfunction call in theAttachDocumentmethod is part of thePushPullOptionsstruct. It is consistently used across the codebase, including in test files and theyorkie_server.gofile, aligning with the PR's objectives. There are no indications of unintended side effects.
- The
PushPullfunction calls inyorkie_server.goand other files confirm the consistent use of the boolean parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the new boolean parameter on the `PushPull` function. # Test: Search for the `PushPull` function definition and its usage across the project. rg --type go -A 5 $'func PushPull'Length of output: 723
Script:
#!/bin/bash # Description: Search for the usage of `PushPull` across the codebase to identify where the boolean parameter is being passed. # Test: Search for `PushPull` function calls across the project to find where the boolean parameter is used. rg --type go 'PushPull(' -A 3Length of output: 100
Script:
#!/bin/bash # Description: Search for the usage of `PushPull` across the codebase to identify where the boolean parameter is being passed. # Test: Correct the regex pattern and search for `PushPull` function calls across the project. rg --type go 'PushPull\(' -A 3Length of output: 19928
549-549: Verify the impact of the new boolean parameter inPushPull.The addition of the
falseparameter to thePushPullfunction call inRemoveDocumentmethod is consistent with the PR's objective to introduce a consistency test. It's important to verify that this change aligns with the expected behavior of thePushPullfunction and does not introduce any unintended side effects.Verification successful
Verified: The boolean parameter in
PushPullis correctly implemented.The
PushPullfunction inserver/packs/packs.goincludes a boolean parametercpTest, which is correctly set tofalsein theRemoveDocumentmethod. This change aligns with the function's definition and does not introduce any inconsistencies. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the new boolean parameter on the `PushPull` function. # Test: Search for the `PushPull` function definition and its usage across the project. rg --type go -A 5 $'func PushPull'Length of output: 723
Script:
#!/bin/bash # Description: Verify the complete function signature of `PushPull` to check for a boolean parameter. # Test: Search for the complete definition of the `PushPull` function in the codebase. ast-grep --lang go --pattern $'func PushPull($_, $_, $_, $_, $_, $_, $_) { $$$ }'Length of output: 84
Script:
#!/bin/bash # Description: Capture the full function signature of `PushPull` to verify the presence of a boolean parameter. # Test: Use `rg` to search for the `PushPull` function definition and include more lines of context. rg --type go -A 20 'func PushPull' server/packs/packs.goLength of output: 1503
352-352: Verify the impact of the new boolean parameter inPushPull.The addition of the
falseparameter to thePushPullfunction call inPushPullChangesmethod is consistent with the PR's objective to introduce a consistency test. It's important to verify that this change aligns with the expected behavior of thePushPullfunction and does not introduce any unintended side effects.
261-261: Verify the impact of the new boolean parameter inPushPull.The addition of the
falseparameter to thePushPullfunction call inDetachDocumentmethod is consistent with the PR's objective to introduce a consistency test. It's important to verify that this change aligns with the expected behavior of thePushPullfunction and does not introduce any unintended side effects.Verification successful
The boolean parameter in
PushPullaligns with the PR's objectives.The addition of the
falseparameter in theDetachDocumentmethod corresponds to thecpTestparameter in thePushPullfunction. This change is consistent with the PR's objective to introduce a consistency test and does not introduce any unintended side effects. The parameter is used to control specific behavior within the function, likely related to consistency checks.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the new boolean parameter on the `PushPull` function. # Test: Search for the `PushPull` function definition and its usage across the project. rg --type go -A 5 $'func PushPull'Length of output: 723
Script:
#!/bin/bash # Extract the complete implementation of the `PushPull` function to understand the usage of the boolean parameter. ast-grep --lang go --pattern $'func PushPull($_, $_, $_, $_, $_, $_, $_) { $$$ }'Length of output: 84
Script:
#!/bin/bash # Use rg to search for the `PushPull` function definition and include more context lines to capture the full implementation. rg --type go -A 20 'func PushPull'Length of output: 2405
server/packs/packs.go
Outdated
| @@ -37,6 +38,10 @@ | |||
| "github.com/yorkie-team/yorkie/server/logging" | |||
| ) | |||
|
|
|||
| var ( | |||
| ErrCheckpointTest = errors.New("failure for checkpoint testing purpose") | |||
There was a problem hiding this comment.
Add a comment for the exported variable ErrCheckpointTest.
The static analysis tool has flagged this line because exported variables in Go should have a comment describing their purpose. This is a best practice to maintain code readability and documentation.
Suggest adding a comment above the variable declaration:
// ErrCheckpointTest is an error used to simulate a failure for checkpoint testing purposes.
var ErrCheckpointTest = errors.New("failure for checkpoint testing purpose")Tools
GitHub Check: build
[failure] 42-42:
exported: exported var ErrCheckpointTest should have comment or be unexported (revive)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Go Benchmark
| Benchmark suite | Current: dfc2843 | Previous: d78f6ff | Ratio |
|---|---|---|---|
BenchmarkDocument/constructor_test |
1475 ns/op 1337 B/op 24 allocs/op |
1643 ns/op 1337 B/op 24 allocs/op |
0.90 |
BenchmarkDocument/constructor_test - ns/op |
1475 ns/op |
1643 ns/op |
0.90 |
BenchmarkDocument/constructor_test - B/op |
1337 B/op |
1337 B/op |
1 |
BenchmarkDocument/constructor_test - allocs/op |
24 allocs/op |
24 allocs/op |
1 |
BenchmarkDocument/status_test |
973 ns/op 1305 B/op 22 allocs/op |
1138 ns/op 1305 B/op 22 allocs/op |
0.86 |
BenchmarkDocument/status_test - ns/op |
973 ns/op |
1138 ns/op |
0.86 |
BenchmarkDocument/status_test - B/op |
1305 B/op |
1305 B/op |
1 |
BenchmarkDocument/status_test - allocs/op |
22 allocs/op |
22 allocs/op |
1 |
BenchmarkDocument/equals_test |
7716 ns/op 7273 B/op 132 allocs/op |
7708 ns/op 7273 B/op 132 allocs/op |
1.00 |
BenchmarkDocument/equals_test - ns/op |
7716 ns/op |
7708 ns/op |
1.00 |
BenchmarkDocument/equals_test - B/op |
7273 B/op |
7273 B/op |
1 |
BenchmarkDocument/equals_test - allocs/op |
132 allocs/op |
132 allocs/op |
1 |
BenchmarkDocument/nested_update_test |
17739 ns/op 12138 B/op 262 allocs/op |
17103 ns/op 12139 B/op 262 allocs/op |
1.04 |
BenchmarkDocument/nested_update_test - ns/op |
17739 ns/op |
17103 ns/op |
1.04 |
BenchmarkDocument/nested_update_test - B/op |
12138 B/op |
12139 B/op |
1.00 |
BenchmarkDocument/nested_update_test - allocs/op |
262 allocs/op |
262 allocs/op |
1 |
BenchmarkDocument/delete_test |
23824 ns/op 15363 B/op 341 allocs/op |
23276 ns/op 15364 B/op 341 allocs/op |
1.02 |
BenchmarkDocument/delete_test - ns/op |
23824 ns/op |
23276 ns/op |
1.02 |
BenchmarkDocument/delete_test - B/op |
15363 B/op |
15364 B/op |
1.00 |
BenchmarkDocument/delete_test - allocs/op |
341 allocs/op |
341 allocs/op |
1 |
BenchmarkDocument/object_test |
9702 ns/op 6817 B/op 120 allocs/op |
8813 ns/op 6817 B/op 120 allocs/op |
1.10 |
BenchmarkDocument/object_test - ns/op |
9702 ns/op |
8813 ns/op |
1.10 |
BenchmarkDocument/object_test - B/op |
6817 B/op |
6817 B/op |
1 |
BenchmarkDocument/object_test - allocs/op |
120 allocs/op |
120 allocs/op |
1 |
BenchmarkDocument/array_test |
29403 ns/op 11947 B/op 276 allocs/op |
29863 ns/op 11947 B/op 276 allocs/op |
0.98 |
BenchmarkDocument/array_test - ns/op |
29403 ns/op |
29863 ns/op |
0.98 |
BenchmarkDocument/array_test - B/op |
11947 B/op |
11947 B/op |
1 |
BenchmarkDocument/array_test - allocs/op |
276 allocs/op |
276 allocs/op |
1 |
BenchmarkDocument/text_test |
30415 ns/op 14715 B/op 469 allocs/op |
31338 ns/op 14715 B/op 469 allocs/op |
0.97 |
BenchmarkDocument/text_test - ns/op |
30415 ns/op |
31338 ns/op |
0.97 |
BenchmarkDocument/text_test - B/op |
14715 B/op |
14715 B/op |
1 |
BenchmarkDocument/text_test - allocs/op |
469 allocs/op |
469 allocs/op |
1 |
BenchmarkDocument/text_composition_test |
28731 ns/op 18422 B/op 484 allocs/op |
29476 ns/op 18420 B/op 484 allocs/op |
0.97 |
BenchmarkDocument/text_composition_test - ns/op |
28731 ns/op |
29476 ns/op |
0.97 |
BenchmarkDocument/text_composition_test - B/op |
18422 B/op |
18420 B/op |
1.00 |
BenchmarkDocument/text_composition_test - allocs/op |
484 allocs/op |
484 allocs/op |
1 |
BenchmarkDocument/rich_text_test |
80892 ns/op 38476 B/op 1148 allocs/op |
83194 ns/op 38476 B/op 1148 allocs/op |
0.97 |
BenchmarkDocument/rich_text_test - ns/op |
80892 ns/op |
83194 ns/op |
0.97 |
BenchmarkDocument/rich_text_test - B/op |
38476 B/op |
38476 B/op |
1 |
BenchmarkDocument/rich_text_test - allocs/op |
1148 allocs/op |
1148 allocs/op |
1 |
BenchmarkDocument/counter_test |
17377 ns/op 10722 B/op 244 allocs/op |
17907 ns/op 10722 B/op 244 allocs/op |
0.97 |
BenchmarkDocument/counter_test - ns/op |
17377 ns/op |
17907 ns/op |
0.97 |
BenchmarkDocument/counter_test - B/op |
10722 B/op |
10722 B/op |
1 |
BenchmarkDocument/counter_test - allocs/op |
244 allocs/op |
244 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_100 |
1272933 ns/op 870930 B/op 16753 allocs/op |
1307290 ns/op 870954 B/op 16753 allocs/op |
0.97 |
BenchmarkDocument/text_edit_gc_100 - ns/op |
1272933 ns/op |
1307290 ns/op |
0.97 |
BenchmarkDocument/text_edit_gc_100 - B/op |
870930 B/op |
870954 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_100 - allocs/op |
16753 allocs/op |
16753 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_1000 |
48982992 ns/op 50536836 B/op 181707 allocs/op |
51215920 ns/op 50535764 B/op 181714 allocs/op |
0.96 |
BenchmarkDocument/text_edit_gc_1000 - ns/op |
48982992 ns/op |
51215920 ns/op |
0.96 |
BenchmarkDocument/text_edit_gc_1000 - B/op |
50536836 B/op |
50535764 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_1000 - allocs/op |
181707 allocs/op |
181714 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_100 |
1867912 ns/op 1528771 B/op 15603 allocs/op |
1930570 ns/op 1528767 B/op 15604 allocs/op |
0.97 |
BenchmarkDocument/text_split_gc_100 - ns/op |
1867912 ns/op |
1930570 ns/op |
0.97 |
BenchmarkDocument/text_split_gc_100 - B/op |
1528771 B/op |
1528767 B/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - allocs/op |
15603 allocs/op |
15604 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 |
110297027 ns/op 135075766 B/op 182203 allocs/op |
118077605 ns/op 135074831 B/op 182188 allocs/op |
0.93 |
BenchmarkDocument/text_split_gc_1000 - ns/op |
110297027 ns/op |
118077605 ns/op |
0.93 |
BenchmarkDocument/text_split_gc_1000 - B/op |
135075766 B/op |
135074831 B/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 - allocs/op |
182203 allocs/op |
182188 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 |
15471309 ns/op 10184728 B/op 40680 allocs/op |
19099277 ns/op 10182896 B/op 40673 allocs/op |
0.81 |
BenchmarkDocument/text_delete_all_10000 - ns/op |
15471309 ns/op |
19099277 ns/op |
0.81 |
BenchmarkDocument/text_delete_all_10000 - B/op |
10184728 B/op |
10182896 B/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 - allocs/op |
40680 allocs/op |
40673 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 |
302144207 ns/op 142667268 B/op 411679 allocs/op |
338994099 ns/op 142684728 B/op 411709 allocs/op |
0.89 |
BenchmarkDocument/text_delete_all_100000 - ns/op |
302144207 ns/op |
338994099 ns/op |
0.89 |
BenchmarkDocument/text_delete_all_100000 - B/op |
142667268 B/op |
142684728 B/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 - allocs/op |
411679 allocs/op |
411709 allocs/op |
1.00 |
BenchmarkDocument/text_100 |
211524 ns/op 120035 B/op 5081 allocs/op |
216689 ns/op 120037 B/op 5081 allocs/op |
0.98 |
BenchmarkDocument/text_100 - ns/op |
211524 ns/op |
216689 ns/op |
0.98 |
BenchmarkDocument/text_100 - B/op |
120035 B/op |
120037 B/op |
1.00 |
BenchmarkDocument/text_100 - allocs/op |
5081 allocs/op |
5081 allocs/op |
1 |
BenchmarkDocument/text_1000 |
2297254 ns/op 1169021 B/op 50085 allocs/op |
2373362 ns/op 1169025 B/op 50085 allocs/op |
0.97 |
BenchmarkDocument/text_1000 - ns/op |
2297254 ns/op |
2373362 ns/op |
0.97 |
BenchmarkDocument/text_1000 - B/op |
1169021 B/op |
1169025 B/op |
1.00 |
BenchmarkDocument/text_1000 - allocs/op |
50085 allocs/op |
50085 allocs/op |
1 |
BenchmarkDocument/array_1000 |
1201006 ns/op 1091345 B/op 11831 allocs/op |
1245310 ns/op 1091393 B/op 11831 allocs/op |
0.96 |
BenchmarkDocument/array_1000 - ns/op |
1201006 ns/op |
1245310 ns/op |
0.96 |
BenchmarkDocument/array_1000 - B/op |
1091345 B/op |
1091393 B/op |
1.00 |
BenchmarkDocument/array_1000 - allocs/op |
11831 allocs/op |
11831 allocs/op |
1 |
BenchmarkDocument/array_10000 |
13047378 ns/op 9800096 B/op 120296 allocs/op |
13759385 ns/op 9800832 B/op 120300 allocs/op |
0.95 |
BenchmarkDocument/array_10000 - ns/op |
13047378 ns/op |
13759385 ns/op |
0.95 |
BenchmarkDocument/array_10000 - B/op |
9800096 B/op |
9800832 B/op |
1.00 |
BenchmarkDocument/array_10000 - allocs/op |
120296 allocs/op |
120300 allocs/op |
1.00 |
BenchmarkDocument/array_gc_100 |
145176 ns/op 132721 B/op 1260 allocs/op |
150829 ns/op 132706 B/op 1260 allocs/op |
0.96 |
BenchmarkDocument/array_gc_100 - ns/op |
145176 ns/op |
150829 ns/op |
0.96 |
BenchmarkDocument/array_gc_100 - B/op |
132721 B/op |
132706 B/op |
1.00 |
BenchmarkDocument/array_gc_100 - allocs/op |
1260 allocs/op |
1260 allocs/op |
1 |
BenchmarkDocument/array_gc_1000 |
1379286 ns/op 1159163 B/op 12877 allocs/op |
1432968 ns/op 1159122 B/op 12876 allocs/op |
0.96 |
BenchmarkDocument/array_gc_1000 - ns/op |
1379286 ns/op |
1432968 ns/op |
0.96 |
BenchmarkDocument/array_gc_1000 - B/op |
1159163 B/op |
1159122 B/op |
1.00 |
BenchmarkDocument/array_gc_1000 - allocs/op |
12877 allocs/op |
12876 allocs/op |
1.00 |
BenchmarkDocument/counter_1000 |
195780 ns/op 193081 B/op 5771 allocs/op |
200690 ns/op 193080 B/op 5771 allocs/op |
0.98 |
BenchmarkDocument/counter_1000 - ns/op |
195780 ns/op |
200690 ns/op |
0.98 |
BenchmarkDocument/counter_1000 - B/op |
193081 B/op |
193080 B/op |
1.00 |
BenchmarkDocument/counter_1000 - allocs/op |
5771 allocs/op |
5771 allocs/op |
1 |
BenchmarkDocument/counter_10000 |
2129554 ns/op 2087995 B/op 59778 allocs/op |
2189662 ns/op 2087996 B/op 59778 allocs/op |
0.97 |
BenchmarkDocument/counter_10000 - ns/op |
2129554 ns/op |
2189662 ns/op |
0.97 |
BenchmarkDocument/counter_10000 - B/op |
2087995 B/op |
2087996 B/op |
1.00 |
BenchmarkDocument/counter_10000 - allocs/op |
59778 allocs/op |
59778 allocs/op |
1 |
BenchmarkDocument/object_1000 |
1366347 ns/op 1427984 B/op 9848 allocs/op |
1405642 ns/op 1427942 B/op 9848 allocs/op |
0.97 |
BenchmarkDocument/object_1000 - ns/op |
1366347 ns/op |
1405642 ns/op |
0.97 |
BenchmarkDocument/object_1000 - B/op |
1427984 B/op |
1427942 B/op |
1.00 |
BenchmarkDocument/object_1000 - allocs/op |
9848 allocs/op |
9848 allocs/op |
1 |
BenchmarkDocument/object_10000 |
14980937 ns/op 12165753 B/op 100562 allocs/op |
15831833 ns/op 12167669 B/op 100566 allocs/op |
0.95 |
BenchmarkDocument/object_10000 - ns/op |
14980937 ns/op |
15831833 ns/op |
0.95 |
BenchmarkDocument/object_10000 - B/op |
12165753 B/op |
12167669 B/op |
1.00 |
BenchmarkDocument/object_10000 - allocs/op |
100562 allocs/op |
100566 allocs/op |
1.00 |
BenchmarkDocument/tree_100 |
1020729 ns/op 943708 B/op 6101 allocs/op |
1035543 ns/op 943700 B/op 6101 allocs/op |
0.99 |
BenchmarkDocument/tree_100 - ns/op |
1020729 ns/op |
1035543 ns/op |
0.99 |
BenchmarkDocument/tree_100 - B/op |
943708 B/op |
943700 B/op |
1.00 |
BenchmarkDocument/tree_100 - allocs/op |
6101 allocs/op |
6101 allocs/op |
1 |
BenchmarkDocument/tree_1000 |
73222795 ns/op 86460329 B/op 60115 allocs/op |
75181078 ns/op 86460275 B/op 60114 allocs/op |
0.97 |
BenchmarkDocument/tree_1000 - ns/op |
73222795 ns/op |
75181078 ns/op |
0.97 |
BenchmarkDocument/tree_1000 - B/op |
86460329 B/op |
86460275 B/op |
1.00 |
BenchmarkDocument/tree_1000 - allocs/op |
60115 allocs/op |
60114 allocs/op |
1.00 |
BenchmarkDocument/tree_10000 |
9219897014 ns/op 8580670864 B/op 600225 allocs/op |
10059128126 ns/op 8580670080 B/op 600236 allocs/op |
0.92 |
BenchmarkDocument/tree_10000 - ns/op |
9219897014 ns/op |
10059128126 ns/op |
0.92 |
BenchmarkDocument/tree_10000 - B/op |
8580670864 B/op |
8580670080 B/op |
1.00 |
BenchmarkDocument/tree_10000 - allocs/op |
600225 allocs/op |
600236 allocs/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 |
73515042 ns/op 87509338 B/op 75264 allocs/op |
77316427 ns/op 87508958 B/op 75263 allocs/op |
0.95 |
BenchmarkDocument/tree_delete_all_1000 - ns/op |
73515042 ns/op |
77316427 ns/op |
0.95 |
BenchmarkDocument/tree_delete_all_1000 - B/op |
87509338 B/op |
87508958 B/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 - allocs/op |
75264 allocs/op |
75263 allocs/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 |
3718177 ns/op 4147735 B/op 15140 allocs/op |
3894522 ns/op 4147766 B/op 15140 allocs/op |
0.95 |
BenchmarkDocument/tree_edit_gc_100 - ns/op |
3718177 ns/op |
3894522 ns/op |
0.95 |
BenchmarkDocument/tree_edit_gc_100 - B/op |
4147735 B/op |
4147766 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 - allocs/op |
15140 allocs/op |
15140 allocs/op |
1 |
BenchmarkDocument/tree_edit_gc_1000 |
295271008 ns/op 383745498 B/op 154856 allocs/op |
313079092 ns/op 383744158 B/op 154847 allocs/op |
0.94 |
BenchmarkDocument/tree_edit_gc_1000 - ns/op |
295271008 ns/op |
313079092 ns/op |
0.94 |
BenchmarkDocument/tree_edit_gc_1000 - B/op |
383745498 B/op |
383744158 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_1000 - allocs/op |
154856 allocs/op |
154847 allocs/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 |
2480531 ns/op 2412517 B/op 11125 allocs/op |
2601492 ns/op 2412507 B/op 11125 allocs/op |
0.95 |
BenchmarkDocument/tree_split_gc_100 - ns/op |
2480531 ns/op |
2601492 ns/op |
0.95 |
BenchmarkDocument/tree_split_gc_100 - B/op |
2412517 B/op |
2412507 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 - allocs/op |
11125 allocs/op |
11125 allocs/op |
1 |
BenchmarkDocument/tree_split_gc_1000 |
180387258 ns/op 222250672 B/op 121992 allocs/op |
190723724 ns/op 222249610 B/op 121982 allocs/op |
0.95 |
BenchmarkDocument/tree_split_gc_1000 - ns/op |
180387258 ns/op |
190723724 ns/op |
0.95 |
BenchmarkDocument/tree_split_gc_1000 - B/op |
222250672 B/op |
222249610 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_1000 - allocs/op |
121992 allocs/op |
121982 allocs/op |
1.00 |
BenchmarkRPC/client_to_server |
343796808 ns/op 16598117 B/op 169205 allocs/op |
363576569 ns/op 16308893 B/op 167370 allocs/op |
0.95 |
BenchmarkRPC/client_to_server - ns/op |
343796808 ns/op |
363576569 ns/op |
0.95 |
BenchmarkRPC/client_to_server - B/op |
16598117 B/op |
16308893 B/op |
1.02 |
BenchmarkRPC/client_to_server - allocs/op |
169205 allocs/op |
167370 allocs/op |
1.01 |
BenchmarkRPC/client_to_client_via_server |
630721665 ns/op 34069116 B/op 324290 allocs/op |
655895366 ns/op 31885260 B/op 321202 allocs/op |
0.96 |
BenchmarkRPC/client_to_client_via_server - ns/op |
630721665 ns/op |
655895366 ns/op |
0.96 |
BenchmarkRPC/client_to_client_via_server - B/op |
34069116 B/op |
31885260 B/op |
1.07 |
BenchmarkRPC/client_to_client_via_server - allocs/op |
324290 allocs/op |
321202 allocs/op |
1.01 |
BenchmarkRPC/attach_large_document |
1274062172 ns/op 1919913472 B/op 8870 allocs/op |
1251900263 ns/op 1896314016 B/op 8822 allocs/op |
1.02 |
BenchmarkRPC/attach_large_document - ns/op |
1274062172 ns/op |
1251900263 ns/op |
1.02 |
BenchmarkRPC/attach_large_document - B/op |
1919913472 B/op |
1896314016 B/op |
1.01 |
BenchmarkRPC/attach_large_document - allocs/op |
8870 allocs/op |
8822 allocs/op |
1.01 |
BenchmarkRPC/adminCli_to_server |
547868414 ns/op 35959644 B/op 289567 allocs/op |
565605350 ns/op 36778368 B/op 289574 allocs/op |
0.97 |
BenchmarkRPC/adminCli_to_server - ns/op |
547868414 ns/op |
565605350 ns/op |
0.97 |
BenchmarkRPC/adminCli_to_server - B/op |
35959644 B/op |
36778368 B/op |
0.98 |
BenchmarkRPC/adminCli_to_server - allocs/op |
289567 allocs/op |
289574 allocs/op |
1.00 |
BenchmarkLocker |
63.29 ns/op 16 B/op 1 allocs/op |
64.44 ns/op 16 B/op 1 allocs/op |
0.98 |
BenchmarkLocker - ns/op |
63.29 ns/op |
64.44 ns/op |
0.98 |
BenchmarkLocker - B/op |
16 B/op |
16 B/op |
1 |
BenchmarkLocker - allocs/op |
1 allocs/op |
1 allocs/op |
1 |
BenchmarkLockerParallel |
39.44 ns/op 0 B/op 0 allocs/op |
39.6 ns/op 0 B/op 0 allocs/op |
1.00 |
BenchmarkLockerParallel - ns/op |
39.44 ns/op |
39.6 ns/op |
1.00 |
BenchmarkLockerParallel - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkLockerParallel - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkLockerMoreKeys |
140.4 ns/op 15 B/op 0 allocs/op |
155.1 ns/op 15 B/op 0 allocs/op |
0.91 |
BenchmarkLockerMoreKeys - ns/op |
140.4 ns/op |
155.1 ns/op |
0.91 |
BenchmarkLockerMoreKeys - B/op |
15 B/op |
15 B/op |
1 |
BenchmarkLockerMoreKeys - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkChange/Push_10_Changes |
3600515 ns/op 118088 B/op 1215 allocs/op |
3743359 ns/op 115560 B/op 1205 allocs/op |
0.96 |
BenchmarkChange/Push_10_Changes - ns/op |
3600515 ns/op |
3743359 ns/op |
0.96 |
BenchmarkChange/Push_10_Changes - B/op |
118088 B/op |
115560 B/op |
1.02 |
BenchmarkChange/Push_10_Changes - allocs/op |
1215 allocs/op |
1205 allocs/op |
1.01 |
BenchmarkChange/Push_100_Changes |
14258876 ns/op 571637 B/op 6585 allocs/op |
14979198 ns/op 568668 B/op 6575 allocs/op |
0.95 |
BenchmarkChange/Push_100_Changes - ns/op |
14258876 ns/op |
14979198 ns/op |
0.95 |
BenchmarkChange/Push_100_Changes - B/op |
571637 B/op |
568668 B/op |
1.01 |
BenchmarkChange/Push_100_Changes - allocs/op |
6585 allocs/op |
6575 allocs/op |
1.00 |
BenchmarkChange/Push_1000_Changes |
116044636 ns/op 5286207 B/op 63076 allocs/op |
119611782 ns/op 5263525 B/op 63070 allocs/op |
0.97 |
BenchmarkChange/Push_1000_Changes - ns/op |
116044636 ns/op |
119611782 ns/op |
0.97 |
BenchmarkChange/Push_1000_Changes - B/op |
5286207 B/op |
5263525 B/op |
1.00 |
BenchmarkChange/Push_1000_Changes - allocs/op |
63076 allocs/op |
63070 allocs/op |
1.00 |
BenchmarkChange/Pull_10_Changes |
2914278 ns/op 103074 B/op 1018 allocs/op |
3043755 ns/op 100564 B/op 1009 allocs/op |
0.96 |
BenchmarkChange/Pull_10_Changes - ns/op |
2914278 ns/op |
3043755 ns/op |
0.96 |
BenchmarkChange/Pull_10_Changes - B/op |
103074 B/op |
100564 B/op |
1.02 |
BenchmarkChange/Pull_10_Changes - allocs/op |
1018 allocs/op |
1009 allocs/op |
1.01 |
BenchmarkChange/Pull_100_Changes |
4380407 ns/op 268569 B/op 3488 allocs/op |
4592542 ns/op 264466 B/op 3479 allocs/op |
0.95 |
BenchmarkChange/Pull_100_Changes - ns/op |
4380407 ns/op |
4592542 ns/op |
0.95 |
BenchmarkChange/Pull_100_Changes - B/op |
268569 B/op |
264466 B/op |
1.02 |
BenchmarkChange/Pull_100_Changes - allocs/op |
3488 allocs/op |
3479 allocs/op |
1.00 |
BenchmarkChange/Pull_1000_Changes |
8625185 ns/op 1496506 B/op 29876 allocs/op |
9224107 ns/op 1490161 B/op 29850 allocs/op |
0.94 |
BenchmarkChange/Pull_1000_Changes - ns/op |
8625185 ns/op |
9224107 ns/op |
0.94 |
BenchmarkChange/Pull_1000_Changes - B/op |
1496506 B/op |
1490161 B/op |
1.00 |
BenchmarkChange/Pull_1000_Changes - allocs/op |
29876 allocs/op |
29850 allocs/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot |
16777298 ns/op 707182 B/op 6586 allocs/op |
17364103 ns/op 714242 B/op 6577 allocs/op |
0.97 |
BenchmarkSnapshot/Push_3KB_snapshot - ns/op |
16777298 ns/op |
17364103 ns/op |
0.97 |
BenchmarkSnapshot/Push_3KB_snapshot - B/op |
707182 B/op |
714242 B/op |
0.99 |
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op |
6586 allocs/op |
6577 allocs/op |
1.00 |
BenchmarkSnapshot/Push_30KB_snapshot |
119417917 ns/op 5593177 B/op 63080 allocs/op |
123110543 ns/op 5601899 B/op 63069 allocs/op |
0.97 |
BenchmarkSnapshot/Push_30KB_snapshot - ns/op |
119417917 ns/op |
123110543 ns/op |
0.97 |
BenchmarkSnapshot/Push_30KB_snapshot - B/op |
5593177 B/op |
5601899 B/op |
1.00 |
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op |
63080 allocs/op |
63069 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot |
6518712 ns/op 925302 B/op 15525 allocs/op |
6724507 ns/op 919082 B/op 15512 allocs/op |
0.97 |
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op |
6518712 ns/op |
6724507 ns/op |
0.97 |
BenchmarkSnapshot/Pull_3KB_snapshot - B/op |
925302 B/op |
919082 B/op |
1.01 |
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op |
15525 allocs/op |
15512 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot |
14745669 ns/op 7165369 B/op 150118 allocs/op |
15655368 ns/op 7152995 B/op 150107 allocs/op |
0.94 |
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op |
14745669 ns/op |
15655368 ns/op |
0.94 |
BenchmarkSnapshot/Pull_30KB_snapshot - B/op |
7165369 B/op |
7152995 B/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op |
150118 allocs/op |
150107 allocs/op |
1.00 |
BenchmarkSync/memory_sync_10_test |
6855 ns/op 1286 B/op 38 allocs/op |
6960 ns/op 1286 B/op 38 allocs/op |
0.98 |
BenchmarkSync/memory_sync_10_test - ns/op |
6855 ns/op |
6960 ns/op |
0.98 |
BenchmarkSync/memory_sync_10_test - B/op |
1286 B/op |
1286 B/op |
1 |
BenchmarkSync/memory_sync_10_test - allocs/op |
38 allocs/op |
38 allocs/op |
1 |
BenchmarkSync/memory_sync_100_test |
51870 ns/op 8629 B/op 272 allocs/op |
58578 ns/op 8907 B/op 290 allocs/op |
0.89 |
BenchmarkSync/memory_sync_100_test - ns/op |
51870 ns/op |
58578 ns/op |
0.89 |
BenchmarkSync/memory_sync_100_test - B/op |
8629 B/op |
8907 B/op |
0.97 |
BenchmarkSync/memory_sync_100_test - allocs/op |
272 allocs/op |
290 allocs/op |
0.94 |
BenchmarkSync/memory_sync_1000_test |
583675 ns/op 74318 B/op 2117 allocs/op |
424052 ns/op 83158 B/op 2671 allocs/op |
1.38 |
BenchmarkSync/memory_sync_1000_test - ns/op |
583675 ns/op |
424052 ns/op |
1.38 |
BenchmarkSync/memory_sync_1000_test - B/op |
74318 B/op |
83158 B/op |
0.89 |
BenchmarkSync/memory_sync_1000_test - allocs/op |
2117 allocs/op |
2671 allocs/op |
0.79 |
BenchmarkSync/memory_sync_10000_test |
7214442 ns/op 737719 B/op 20311 allocs/op |
4476569 ns/op 801206 B/op 24164 allocs/op |
1.61 |
BenchmarkSync/memory_sync_10000_test - ns/op |
7214442 ns/op |
4476569 ns/op |
1.61 |
BenchmarkSync/memory_sync_10000_test - B/op |
737719 B/op |
801206 B/op |
0.92 |
BenchmarkSync/memory_sync_10000_test - allocs/op |
20311 allocs/op |
24164 allocs/op |
0.84 |
BenchmarkTextEditing |
5970244397 ns/op 3901902736 B/op 18743145 allocs/op |
5148723113 ns/op 3901867360 B/op 18743050 allocs/op |
1.16 |
BenchmarkTextEditing - ns/op |
5970244397 ns/op |
5148723113 ns/op |
1.16 |
BenchmarkTextEditing - B/op |
3901902736 B/op |
3901867360 B/op |
1.00 |
BenchmarkTextEditing - allocs/op |
18743145 allocs/op |
18743050 allocs/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf |
3488923 ns/op 6262995 B/op 70025 allocs/op |
3613564 ns/op 6263000 B/op 70025 allocs/op |
0.97 |
BenchmarkTree/10000_vertices_to_protobuf - ns/op |
3488923 ns/op |
3613564 ns/op |
0.97 |
BenchmarkTree/10000_vertices_to_protobuf - B/op |
6262995 B/op |
6263000 B/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf - allocs/op |
70025 allocs/op |
70025 allocs/op |
1 |
BenchmarkTree/10000_vertices_from_protobuf |
153424530 ns/op 442171425 B/op 290038 allocs/op |
162129213 ns/op 442171432 B/op 290039 allocs/op |
0.95 |
BenchmarkTree/10000_vertices_from_protobuf - ns/op |
153424530 ns/op |
162129213 ns/op |
0.95 |
BenchmarkTree/10000_vertices_from_protobuf - B/op |
442171425 B/op |
442171432 B/op |
1.00 |
BenchmarkTree/10000_vertices_from_protobuf - allocs/op |
290038 allocs/op |
290039 allocs/op |
1.00 |
BenchmarkTree/20000_vertices_to_protobuf |
7917616 ns/op 12716921 B/op 140028 allocs/op |
7997732 ns/op 12716921 B/op 140028 allocs/op |
0.99 |
BenchmarkTree/20000_vertices_to_protobuf - ns/op |
7917616 ns/op |
7997732 ns/op |
0.99 |
BenchmarkTree/20000_vertices_to_protobuf - B/op |
12716921 B/op |
12716921 B/op |
1 |
BenchmarkTree/20000_vertices_to_protobuf - allocs/op |
140028 allocs/op |
140028 allocs/op |
1 |
BenchmarkTree/20000_vertices_from_protobuf |
684908304 ns/op 1697268320 B/op 580088 allocs/op |
702832234 ns/op 1697272560 B/op 580089 allocs/op |
0.97 |
BenchmarkTree/20000_vertices_from_protobuf - ns/op |
684908304 ns/op |
702832234 ns/op |
0.97 |
BenchmarkTree/20000_vertices_from_protobuf - B/op |
1697268320 B/op |
1697272560 B/op |
1.00 |
BenchmarkTree/20000_vertices_from_protobuf - allocs/op |
580088 allocs/op |
580089 allocs/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf |
12245792 ns/op 19318334 B/op 210030 allocs/op |
12542423 ns/op 19318325 B/op 210030 allocs/op |
0.98 |
BenchmarkTree/30000_vertices_to_protobuf - ns/op |
12245792 ns/op |
12542423 ns/op |
0.98 |
BenchmarkTree/30000_vertices_to_protobuf - B/op |
19318334 B/op |
19318325 B/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf - allocs/op |
210030 allocs/op |
210030 allocs/op |
1 |
BenchmarkTree/30000_vertices_from_protobuf |
1654462225 ns/op 3752052840 B/op 870052 allocs/op |
1666345166 ns/op 3752044440 B/op 870052 allocs/op |
0.99 |
BenchmarkTree/30000_vertices_from_protobuf - ns/op |
1654462225 ns/op |
1666345166 ns/op |
0.99 |
BenchmarkTree/30000_vertices_from_protobuf - B/op |
3752052840 B/op |
3752044440 B/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf - allocs/op |
870052 allocs/op |
870052 allocs/op |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
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)
- server/packs/packs.go (4 hunks)
- server/packs/packs_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- server/packs/packs.go
- server/packs/packs_test.go
|
Related to #1001 . |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1000 +/- ##
==========================================
- Coverage 50.80% 48.68% -2.13%
==========================================
Files 73 79 +6
Lines 10865 11351 +486
==========================================
+ Hits 5520 5526 +6
- Misses 4796 5276 +480
Partials 549 549 ☔ View full report in Codecov by Sentry. |
|
@hackerwins @kokodak |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
Files selected for processing (3)
- go.mod (2 hunks)
- server/packs/mock_db.go (1 hunks)
- server/packs/packs_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- server/packs/packs_test.go
Additional comments not posted (3)
go.mod (2)
21-21: Approved: Dependency update forgithub.com/undefinedlabs/go-mpatch.The update from
v1.0.6tov1.0.7is a minor version change, typically associated with bug fixes or performance improvements. Ensure that this update has been tested to confirm that it does not introduce any breaking changes or regressions.
33-33: Approved with a note to verify: Addition ofgithub.com/stretchr/objx.The addition of
objxas an indirect dependency suggests it's required by another library. Please verify that the licensing ofobjxaligns with the project's requirements and assess any potential impact on the project's size or performance.Verification successful
Verification complete:
github.com/stretchr/objxis licensed under the MIT License.The MIT License is permissive and generally compatible with most projects, so there should be no licensing issues with this dependency.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the licensing of `github.com/stretchr/objx`. # Test: Check the license file in the repository. gh repo view github.com/stretchr/objx --json license --jq '.license'Length of output: 1193
Script:
#!/bin/bash # Description: Verify the licensing of `github.com/stretchr/objx` using the correct field. # Test: Check the licenseInfo field in the repository. gh repo view github.com/stretchr/objx --json licenseInfo --jq '.licenseInfo'Length of output: 125
server/packs/mock_db.go (1)
17-338: Comprehensive Review of Mock Database ImplementationThe new file
mock_db.gointroduces a mock database implementation which is crucial for testing the functionality of the Yorkie project without interacting with the actual database. The use ofmock.Mockfrom thetestifylibrary is appropriate and aligns with common practices in Go for creating test doubles.Each method properly delegates to the real database's corresponding method, which is a good practice ensuring that the mock behaves similarly to the actual database under test conditions. This approach helps in maintaining consistency in test environments and can significantly aid in the detection of issues during the development phase.
Overall, the implementation is clean, follows Go's idiomatic practices, and should serve its intended purpose effectively.
There was a problem hiding this comment.
The test code looks to be well written!
However, there seems to be some problems with the current MockDB as follows:
- When there is a change in the Database interface, MockDB does not satisfy the Database interface, which is easily broken.
- Although it is MockDB, the remaining methods are not stubbable except for the UpdateClientInfoAfterPushPull method.
To properly address this, I've been considering two solutions as follows:
- Embed the Database interface inside the MockDB struct, inject the MongoDB implementation externally, and override only the methods that require stubbing. In this case, I think we can stub only the necessary methods while minimising the boilerplate code.
- Use Mockery to automatically generate mocks for the Database interface, and put the actual DB logic needed by the test inside the stubbing logic. In this case, I think partial mocking would be possible.
In my opinion, option 1 would be a good way to improve the problem, because it doesn't rely on external libraries and minimises boilerplate code.
hackerwins
changed the title
The test case simulates a partial failure in PushPull logic, exposing the current limitation in change validation. By implementing this test, we aim to improve the robustness of our system against duplicate changes and maintain data consistency across partial failures. To ensure idempotency of duplicate operations and prevent unintended consequences, we need to implement additional validation beyond ClientSeq. --------- Co-authored-by: kokodak <kokodakadokok@gmail.com>
What this PR does / why we need it:
This PR adds a consistency test for the scenario where updating
ClientInfofails duringPushPull.Which issue(s) this PR fixes:
Related to #1001
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests
packsmodule, validating document change consistency and error handling scenarios.