Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix version vector cleanup during client deactivation #1068

Merged
merged 6 commits into from
Nov 19, 2024
Merged

Fix version vector cleanup during client deactivation #1068

merged 6 commits into from
Nov 19, 2024

Conversation

raararaara
Copy link
Contributor

@raararaara raararaara commented Nov 14, 2024 •
edited by coderabbitai bot
Loading

What this PR does / why we need it?

This PR resolves the issue where the version vector is not being correctly removed during the document detach process caused by client deactivation.

When deactivate is called, the pushpull process within detachDocument properly deletes the version vector. However, after a successful completion of detachDocument, an additional call to UpdateVersionVector mistakenly attempts to upsert the version vector for a client that has already been removed, causing it to appear as though the deletion did not occur.

By removing the call to UpdateVersionVector during the client deactivation process, this issue is effectively resolved.

Any background context you want to provide?

What are the relevant tickets?

Fixes #1058

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Additional documentation:

Checklist:

  • Added relevant tests or not required
  • Didn't break anything

Summary by CodeRabbit

  • Bug Fixes

    • Removed unnecessary updates to document version tracking upon client deactivation to streamline functionality.

  • Tests

    • Enhanced clarity of test assertions related to garbage collection, ensuring explicit comparisons for expected outcomes.
    • Introduced a new test case for garbage collection involving tree structures to validate version vector checks.

@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Nov 14, 2024 •
edited
Loading

Walkthrough

The pull request modifies the Deactivate function in the clients package by removing the code that updates the version vector in the database after detaching documents from a client. This change affects how document versions are tracked post-deactivation. Additionally, the gc_test.go file has been updated to improve the clarity of assertions related to garbage collection, specifically regarding the handling of deactivated clients and their version vectors.

Changes

File Change Summary
server/clients/clients.go Removed the code block that updates the version vector after detaching documents from a client.
test/integration/gc_test.go Updated assertions to explicitly compare the result of checkVV with true for clarity.

Assessment against linked issues

Objective Addressed Explanation
Ensure version vector is removed properly for deactivated clients (#1058) ✅

Possibly related PRs

Warning

Rate limit exceeded

@raararaara has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 47 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 3cf29bf and c7770f2.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3cf29bf and c7770f2.

📒 Files selected for processing (1)
  • test/integration/gc_test.go (32 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/integration/gc_test.go

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov Codecov
Copy link

codecov bot commented Nov 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.82%. Comparing base (f8ebba2) to head (c7770f2).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1068   +/-   ##
=======================================
  Coverage   46.82%   46.82%           
=======================================
  Files          84       84           
  Lines       12180    12180           
=======================================
  Hits         5703     5703           
  Misses       5909     5909           
  Partials      568      568

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@raararaara
Copy link
Contributor Author

raararaara commented Nov 14, 2024 •
edited
Loading

Because the fact that no error occurred during the detach process guarantees that the detach was completed successfully, I manually manipulated clientInfo.

Are there any other problems I might encounter with this?

Since the verson vector is deleted during the pushpull process, this step is unnecessary.

github-actions[bot]
github-actions bot reviewed Nov 19, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go Benchmark

Benchmark suite Current: c7770f2 Previous: 1ca345d Ratio
BenchmarkDocument/constructor_test 1482 ns/op 1337 B/op 24 allocs/op 1480 ns/op 1337 B/op 24 allocs/op 1.00
BenchmarkDocument/constructor_test - ns/op 1482 ns/op 1480 ns/op 1.00
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 1125 ns/op 1305 B/op 22 allocs/op 956.8 ns/op 1305 B/op 22 allocs/op 1.18
BenchmarkDocument/status_test - ns/op 1125 ns/op 956.8 ns/op 1.18
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 7769 ns/op 7529 B/op 134 allocs/op 7807 ns/op 7529 B/op 134 allocs/op 1.00
BenchmarkDocument/equals_test - ns/op 7769 ns/op 7807 ns/op 1.00
BenchmarkDocument/equals_test - B/op 7529 B/op 7529 B/op 1
BenchmarkDocument/equals_test - allocs/op 134 allocs/op 134 allocs/op 1
BenchmarkDocument/nested_update_test 17122 ns/op 12394 B/op 264 allocs/op 19502 ns/op 12395 B/op 264 allocs/op 0.88
BenchmarkDocument/nested_update_test - ns/op 17122 ns/op 19502 ns/op 0.88
BenchmarkDocument/nested_update_test - B/op 12394 B/op 12395 B/op 1.00
BenchmarkDocument/nested_update_test - allocs/op 264 allocs/op 264 allocs/op 1
BenchmarkDocument/delete_test 23151 ns/op 15923 B/op 347 allocs/op 23015 ns/op 15923 B/op 347 allocs/op 1.01
BenchmarkDocument/delete_test - ns/op 23151 ns/op 23015 ns/op 1.01
BenchmarkDocument/delete_test - B/op 15923 B/op 15923 B/op 1
BenchmarkDocument/delete_test - allocs/op 347 allocs/op 347 allocs/op 1
BenchmarkDocument/object_test 8895 ns/op 7073 B/op 122 allocs/op 8837 ns/op 7073 B/op 122 allocs/op 1.01
BenchmarkDocument/object_test - ns/op 8895 ns/op 8837 ns/op 1.01
BenchmarkDocument/object_test - B/op 7073 B/op 7073 B/op 1
BenchmarkDocument/object_test - allocs/op 122 allocs/op 122 allocs/op 1
BenchmarkDocument/array_test 30311 ns/op 12203 B/op 278 allocs/op 29558 ns/op 12203 B/op 278 allocs/op 1.03
BenchmarkDocument/array_test - ns/op 30311 ns/op 29558 ns/op 1.03
BenchmarkDocument/array_test - B/op 12203 B/op 12203 B/op 1
BenchmarkDocument/array_test - allocs/op 278 allocs/op 278 allocs/op 1
BenchmarkDocument/text_test 31870 ns/op 15324 B/op 492 allocs/op 31875 ns/op 15324 B/op 492 allocs/op 1.00
BenchmarkDocument/text_test - ns/op 31870 ns/op 31875 ns/op 1.00
BenchmarkDocument/text_test - B/op 15324 B/op 15324 B/op 1
BenchmarkDocument/text_test - allocs/op 492 allocs/op 492 allocs/op 1
BenchmarkDocument/text_composition_test 30116 ns/op 18718 B/op 504 allocs/op 30245 ns/op 18718 B/op 504 allocs/op 1.00
BenchmarkDocument/text_composition_test - ns/op 30116 ns/op 30245 ns/op 1.00
BenchmarkDocument/text_composition_test - B/op 18718 B/op 18718 B/op 1
BenchmarkDocument/text_composition_test - allocs/op 504 allocs/op 504 allocs/op 1
BenchmarkDocument/rich_text_test 85308 ns/op 40181 B/op 1183 allocs/op 86444 ns/op 40181 B/op 1183 allocs/op 0.99
BenchmarkDocument/rich_text_test - ns/op 85308 ns/op 86444 ns/op 0.99
BenchmarkDocument/rich_text_test - B/op 40181 B/op 40181 B/op 1
BenchmarkDocument/rich_text_test - allocs/op 1183 allocs/op 1183 allocs/op 1
BenchmarkDocument/counter_test 18591 ns/op 11874 B/op 258 allocs/op 18607 ns/op 11874 B/op 258 allocs/op 1.00
BenchmarkDocument/counter_test - ns/op 18591 ns/op 18607 ns/op 1.00
BenchmarkDocument/counter_test - B/op 11874 B/op 11874 B/op 1
BenchmarkDocument/counter_test - allocs/op 258 allocs/op 258 allocs/op 1
BenchmarkDocument/text_edit_gc_100 1306676 ns/op 872560 B/op 17282 allocs/op 1311957 ns/op 872618 B/op 17282 allocs/op 1.00
BenchmarkDocument/text_edit_gc_100 - ns/op 1306676 ns/op 1311957 ns/op 1.00
BenchmarkDocument/text_edit_gc_100 - B/op 872560 B/op 872618 B/op 1.00
BenchmarkDocument/text_edit_gc_100 - allocs/op 17282 allocs/op 17282 allocs/op 1
BenchmarkDocument/text_edit_gc_1000 49755134 ns/op 50546790 B/op 186732 allocs/op 50363092 ns/op 50546876 B/op 186746 allocs/op 0.99
BenchmarkDocument/text_edit_gc_1000 - ns/op 49755134 ns/op 50363092 ns/op 0.99
BenchmarkDocument/text_edit_gc_1000 - B/op 50546790 B/op 50546876 B/op 1.00
BenchmarkDocument/text_edit_gc_1000 - allocs/op 186732 allocs/op 186746 allocs/op 1.00
BenchmarkDocument/text_split_gc_100 1937708 ns/op 1589081 B/op 15951 allocs/op 1927736 ns/op 1589088 B/op 15951 allocs/op 1.01
BenchmarkDocument/text_split_gc_100 - ns/op 1937708 ns/op 1927736 ns/op 1.01
BenchmarkDocument/text_split_gc_100 - B/op 1589081 B/op 1589088 B/op 1.00
BenchmarkDocument/text_split_gc_100 - allocs/op 15951 allocs/op 15951 allocs/op 1
BenchmarkDocument/text_split_gc_1000 115256186 ns/op 141482610 B/op 186139 allocs/op 117075210 ns/op 141482976 B/op 186139 allocs/op 0.98
BenchmarkDocument/text_split_gc_1000 - ns/op 115256186 ns/op 117075210 ns/op 0.98
BenchmarkDocument/text_split_gc_1000 - B/op 141482610 B/op 141482976 B/op 1.00
BenchmarkDocument/text_split_gc_1000 - allocs/op 186139 allocs/op 186139 allocs/op 1
BenchmarkDocument/text_delete_all_10000 15789999 ns/op 10213554 B/op 55686 allocs/op 16416736 ns/op 10212434 B/op 55682 allocs/op 0.96
BenchmarkDocument/text_delete_all_10000 - ns/op 15789999 ns/op 16416736 ns/op 0.96
BenchmarkDocument/text_delete_all_10000 - B/op 10213554 B/op 10212434 B/op 1.00
BenchmarkDocument/text_delete_all_10000 - allocs/op 55686 allocs/op 55682 allocs/op 1.00
BenchmarkDocument/text_delete_all_100000 284759728 ns/op 142948004 B/op 561640 allocs/op 286805604 ns/op 142965468 B/op 561630 allocs/op 0.99
BenchmarkDocument/text_delete_all_100000 - ns/op 284759728 ns/op 286805604 ns/op 0.99
BenchmarkDocument/text_delete_all_100000 - B/op 142948004 B/op 142965468 B/op 1.00
BenchmarkDocument/text_delete_all_100000 - allocs/op 561640 allocs/op 561630 allocs/op 1.00
BenchmarkDocument/text_100 233275 ns/op 120491 B/op 5182 allocs/op 222738 ns/op 120491 B/op 5182 allocs/op 1.05
BenchmarkDocument/text_100 - ns/op 233275 ns/op 222738 ns/op 1.05
BenchmarkDocument/text_100 - B/op 120491 B/op 120491 B/op 1
BenchmarkDocument/text_100 - allocs/op 5182 allocs/op 5182 allocs/op 1
BenchmarkDocument/text_1000 2492687 ns/op 1171263 B/op 51086 allocs/op 2419254 ns/op 1171262 B/op 51086 allocs/op 1.03
BenchmarkDocument/text_1000 - ns/op 2492687 ns/op 2419254 ns/op 1.03
BenchmarkDocument/text_1000 - B/op 1171263 B/op 1171262 B/op 1.00
BenchmarkDocument/text_1000 - allocs/op 51086 allocs/op 51086 allocs/op 1
BenchmarkDocument/array_1000 1252320 ns/op 1091603 B/op 11833 allocs/op 1205391 ns/op 1091620 B/op 11833 allocs/op 1.04
BenchmarkDocument/array_1000 - ns/op 1252320 ns/op 1205391 ns/op 1.04
BenchmarkDocument/array_1000 - B/op 1091603 B/op 1091620 B/op 1.00
BenchmarkDocument/array_1000 - allocs/op 11833 allocs/op 11833 allocs/op 1
BenchmarkDocument/array_10000 13210996 ns/op 9799607 B/op 120296 allocs/op 13200359 ns/op 9800295 B/op 120298 allocs/op 1.00
BenchmarkDocument/array_10000 - ns/op 13210996 ns/op 13200359 ns/op 1.00
BenchmarkDocument/array_10000 - B/op 9799607 B/op 9800295 B/op 1.00
BenchmarkDocument/array_10000 - allocs/op 120296 allocs/op 120298 allocs/op 1.00
BenchmarkDocument/array_gc_100 155812 ns/op 133277 B/op 1266 allocs/op 146884 ns/op 133276 B/op 1266 allocs/op 1.06
BenchmarkDocument/array_gc_100 - ns/op 155812 ns/op 146884 ns/op 1.06
BenchmarkDocument/array_gc_100 - B/op 133277 B/op 133276 B/op 1.00
BenchmarkDocument/array_gc_100 - allocs/op 1266 allocs/op 1266 allocs/op 1
BenchmarkDocument/array_gc_1000 1441557 ns/op 1159594 B/op 12882 allocs/op 1381037 ns/op 1159720 B/op 12883 allocs/op 1.04
BenchmarkDocument/array_gc_1000 - ns/op 1441557 ns/op 1381037 ns/op 1.04
BenchmarkDocument/array_gc_1000 - B/op 1159594 B/op 1159720 B/op 1.00
BenchmarkDocument/array_gc_1000 - allocs/op 12882 allocs/op 12883 allocs/op 1.00
BenchmarkDocument/counter_1000 214297 ns/op 193336 B/op 5773 allocs/op 204042 ns/op 193337 B/op 5773 allocs/op 1.05
BenchmarkDocument/counter_1000 - ns/op 214297 ns/op 204042 ns/op 1.05
BenchmarkDocument/counter_1000 - B/op 193336 B/op 193337 B/op 1.00
BenchmarkDocument/counter_1000 - allocs/op 5773 allocs/op 5773 allocs/op 1
BenchmarkDocument/counter_10000 2227665 ns/op 2088254 B/op 59780 allocs/op 2208633 ns/op 2088253 B/op 59780 allocs/op 1.01
BenchmarkDocument/counter_10000 - ns/op 2227665 ns/op 2208633 ns/op 1.01
BenchmarkDocument/counter_10000 - B/op 2088254 B/op 2088253 B/op 1.00
BenchmarkDocument/counter_10000 - allocs/op 59780 allocs/op 59780 allocs/op 1
BenchmarkDocument/object_1000 1436181 ns/op 1428312 B/op 9851 allocs/op 1370916 ns/op 1428317 B/op 9851 allocs/op 1.05
BenchmarkDocument/object_1000 - ns/op 1436181 ns/op 1370916 ns/op 1.05
BenchmarkDocument/object_1000 - B/op 1428312 B/op 1428317 B/op 1.00
BenchmarkDocument/object_1000 - allocs/op 9851 allocs/op 9851 allocs/op 1
BenchmarkDocument/object_10000 14954068 ns/op 12167637 B/op 100570 allocs/op 15324520 ns/op 12168220 B/op 100569 allocs/op 0.98
BenchmarkDocument/object_10000 - ns/op 14954068 ns/op 15324520 ns/op 0.98
BenchmarkDocument/object_10000 - B/op 12167637 B/op 12168220 B/op 1.00
BenchmarkDocument/object_10000 - allocs/op 100570 allocs/op 100569 allocs/op 1.00
BenchmarkDocument/tree_100 1076754 ns/op 943958 B/op 6103 allocs/op 1070763 ns/op 943958 B/op 6103 allocs/op 1.01
BenchmarkDocument/tree_100 - ns/op 1076754 ns/op 1070763 ns/op 1.01
BenchmarkDocument/tree_100 - B/op 943958 B/op 943958 B/op 1
BenchmarkDocument/tree_100 - allocs/op 6103 allocs/op 6103 allocs/op 1
BenchmarkDocument/tree_1000 77244402 ns/op 86461133 B/op 60117 allocs/op 77176824 ns/op 86460125 B/op 60117 allocs/op 1.00
BenchmarkDocument/tree_1000 - ns/op 77244402 ns/op 77176824 ns/op 1.00
BenchmarkDocument/tree_1000 - B/op 86461133 B/op 86460125 B/op 1.00
BenchmarkDocument/tree_1000 - allocs/op 60117 allocs/op 60117 allocs/op 1
BenchmarkDocument/tree_10000 9453849955 ns/op 8580662784 B/op 600236 allocs/op 9248587171 ns/op 8580670032 B/op 600232 allocs/op 1.02
BenchmarkDocument/tree_10000 - ns/op 9453849955 ns/op 9248587171 ns/op 1.02
BenchmarkDocument/tree_10000 - B/op 8580662784 B/op 8580670032 B/op 1.00
BenchmarkDocument/tree_10000 - allocs/op 600236 allocs/op 600232 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 78703272 ns/op 87533040 B/op 75270 allocs/op 74364394 ns/op 87510013 B/op 75270 allocs/op 1.06
BenchmarkDocument/tree_delete_all_1000 - ns/op 78703272 ns/op 74364394 ns/op 1.06
BenchmarkDocument/tree_delete_all_1000 - B/op 87533040 B/op 87510013 B/op 1.00
BenchmarkDocument/tree_delete_all_1000 - allocs/op 75270 allocs/op 75270 allocs/op 1
BenchmarkDocument/tree_edit_gc_100 3958439 ns/op 4147178 B/op 15146 allocs/op 3737095 ns/op 4148319 B/op 15147 allocs/op 1.06
BenchmarkDocument/tree_edit_gc_100 - ns/op 3958439 ns/op 3737095 ns/op 1.06
BenchmarkDocument/tree_edit_gc_100 - B/op 4147178 B/op 4148319 B/op 1.00
BenchmarkDocument/tree_edit_gc_100 - allocs/op 15146 allocs/op 15147 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_1000 323985525 ns/op 383746074 B/op 154865 allocs/op 294779034 ns/op 383746786 B/op 154859 allocs/op 1.10
BenchmarkDocument/tree_edit_gc_1000 - ns/op 323985525 ns/op 294779034 ns/op 1.10
BenchmarkDocument/tree_edit_gc_1000 - B/op 383746074 B/op 383746786 B/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - allocs/op 154865 allocs/op 154859 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 2694872 ns/op 2413136 B/op 11131 allocs/op 2537282 ns/op 2413075 B/op 11131 allocs/op 1.06
BenchmarkDocument/tree_split_gc_100 - ns/op 2694872 ns/op 2537282 ns/op 1.06
BenchmarkDocument/tree_split_gc_100 - B/op 2413136 B/op 2413075 B/op 1.00
BenchmarkDocument/tree_split_gc_100 - allocs/op 11131 allocs/op 11131 allocs/op 1
BenchmarkDocument/tree_split_gc_1000 192040132 ns/op 222254284 B/op 122014 allocs/op 180042612 ns/op 222251385 B/op 121997 allocs/op 1.07
BenchmarkDocument/tree_split_gc_1000 - ns/op 192040132 ns/op 180042612 ns/op 1.07
BenchmarkDocument/tree_split_gc_1000 - B/op 222254284 B/op 222251385 B/op 1.00
BenchmarkDocument/tree_split_gc_1000 - allocs/op 122014 allocs/op 121997 allocs/op 1.00
BenchmarkRPC/client_to_server 409446261 ns/op 19150296 B/op 203718 allocs/op 404163651 ns/op 20613208 B/op 204638 allocs/op 1.01
BenchmarkRPC/client_to_server - ns/op 409446261 ns/op 404163651 ns/op 1.01
BenchmarkRPC/client_to_server - B/op 19150296 B/op 20613208 B/op 0.93
BenchmarkRPC/client_to_server - allocs/op 203718 allocs/op 204638 allocs/op 1.00
BenchmarkRPC/client_to_client_via_server 747803032 ns/op 41379360 B/op 422102 allocs/op 735116092 ns/op 39259300 B/op 417195 allocs/op 1.02
BenchmarkRPC/client_to_client_via_server - ns/op 747803032 ns/op 735116092 ns/op 1.02
BenchmarkRPC/client_to_client_via_server - B/op 41379360 B/op 39259300 B/op 1.05
BenchmarkRPC/client_to_client_via_server - allocs/op 422102 allocs/op 417195 allocs/op 1.01
BenchmarkRPC/attach_large_document 1305386414 ns/op 1896509336 B/op 11863 allocs/op 1300029896 ns/op 1919356024 B/op 12338 allocs/op 1.00
BenchmarkRPC/attach_large_document - ns/op 1305386414 ns/op 1300029896 ns/op 1.00
BenchmarkRPC/attach_large_document - B/op 1896509336 B/op 1919356024 B/op 0.99
BenchmarkRPC/attach_large_document - allocs/op 11863 allocs/op 12338 allocs/op 0.96
BenchmarkRPC/adminCli_to_server 532036499 ns/op 37636780 B/op 289790 allocs/op 522897257 ns/op 36009000 B/op 289706 allocs/op 1.02
BenchmarkRPC/adminCli_to_server - ns/op 532036499 ns/op 522897257 ns/op 1.02
BenchmarkRPC/adminCli_to_server - B/op 37636780 B/op 36009000 B/op 1.05
BenchmarkRPC/adminCli_to_server - allocs/op 289790 allocs/op 289706 allocs/op 1.00
BenchmarkLocker 66.08 ns/op 16 B/op 1 allocs/op 66.99 ns/op 16 B/op 1 allocs/op 0.99
BenchmarkLocker - ns/op 66.08 ns/op 66.99 ns/op 0.99
BenchmarkLocker - B/op 16 B/op 16 B/op 1
BenchmarkLocker - allocs/op 1 allocs/op 1 allocs/op 1
BenchmarkLockerParallel 39.46 ns/op 0 B/op 0 allocs/op 37.96 ns/op 0 B/op 0 allocs/op 1.04
BenchmarkLockerParallel - ns/op 39.46 ns/op 37.96 ns/op 1.04
BenchmarkLockerParallel - B/op 0 B/op 0 B/op 1
BenchmarkLockerParallel - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkLockerMoreKeys 146.9 ns/op 15 B/op 0 allocs/op 150.2 ns/op 15 B/op 0 allocs/op 0.98
BenchmarkLockerMoreKeys - ns/op 146.9 ns/op 150.2 ns/op 0.98
BenchmarkLockerMoreKeys - B/op 15 B/op 15 B/op 1
BenchmarkLockerMoreKeys - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkChange/Push_10_Changes 4434221 ns/op 144870 B/op 1572 allocs/op 4298559 ns/op 145022 B/op 1571 allocs/op 1.03
BenchmarkChange/Push_10_Changes - ns/op 4434221 ns/op 4298559 ns/op 1.03
BenchmarkChange/Push_10_Changes - B/op 144870 B/op 145022 B/op 1.00
BenchmarkChange/Push_10_Changes - allocs/op 1572 allocs/op 1571 allocs/op 1.00
BenchmarkChange/Push_100_Changes 15701626 ns/op 734999 B/op 8191 allocs/op 15510109 ns/op 734148 B/op 8192 allocs/op 1.01
BenchmarkChange/Push_100_Changes - ns/op 15701626 ns/op 15510109 ns/op 1.01
BenchmarkChange/Push_100_Changes - B/op 734999 B/op 734148 B/op 1.00
BenchmarkChange/Push_100_Changes - allocs/op 8191 allocs/op 8192 allocs/op 1.00
BenchmarkChange/Push_1000_Changes 125944096 ns/op 7092458 B/op 77177 allocs/op 124439583 ns/op 6972429 B/op 77176 allocs/op 1.01
BenchmarkChange/Push_1000_Changes - ns/op 125944096 ns/op 124439583 ns/op 1.01
BenchmarkChange/Push_1000_Changes - B/op 7092458 B/op 6972429 B/op 1.02
BenchmarkChange/Push_1000_Changes - allocs/op 77177 allocs/op 77176 allocs/op 1.00
BenchmarkChange/Pull_10_Changes 3595974 ns/op 124086 B/op 1403 allocs/op 3583468 ns/op 124431 B/op 1403 allocs/op 1.00
BenchmarkChange/Pull_10_Changes - ns/op 3595974 ns/op 3583468 ns/op 1.00
BenchmarkChange/Pull_10_Changes - B/op 124086 B/op 124431 B/op 1.00
BenchmarkChange/Pull_10_Changes - allocs/op 1403 allocs/op 1403 allocs/op 1
BenchmarkChange/Pull_100_Changes 5090789 ns/op 352030 B/op 4949 allocs/op 5131226 ns/op 352725 B/op 4948 allocs/op 0.99
BenchmarkChange/Pull_100_Changes - ns/op 5090789 ns/op 5131226 ns/op 0.99
BenchmarkChange/Pull_100_Changes - B/op 352030 B/op 352725 B/op 1.00
BenchmarkChange/Pull_100_Changes - allocs/op 4949 allocs/op 4948 allocs/op 1.00
BenchmarkChange/Pull_1000_Changes 10222169 ns/op 2219980 B/op 42671 allocs/op 10486285 ns/op 2220526 B/op 42671 allocs/op 0.97
BenchmarkChange/Pull_1000_Changes - ns/op 10222169 ns/op 10486285 ns/op 0.97
BenchmarkChange/Pull_1000_Changes - B/op 2219980 B/op 2220526 B/op 1.00
BenchmarkChange/Pull_1000_Changes - allocs/op 42671 allocs/op 42671 allocs/op 1
BenchmarkSnapshot/Push_3KB_snapshot 17979591 ns/op 861086 B/op 8194 allocs/op 17829496 ns/op 862965 B/op 8195 allocs/op 1.01
BenchmarkSnapshot/Push_3KB_snapshot - ns/op 17979591 ns/op 17829496 ns/op 1.01
BenchmarkSnapshot/Push_3KB_snapshot - B/op 861086 B/op 862965 B/op 1.00
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op 8194 allocs/op 8195 allocs/op 1.00
BenchmarkSnapshot/Push_30KB_snapshot 129002011 ns/op 7808286 B/op 81801 allocs/op 127440610 ns/op 7751132 B/op 80497 allocs/op 1.01
BenchmarkSnapshot/Push_30KB_snapshot - ns/op 129002011 ns/op 127440610 ns/op 1.01
BenchmarkSnapshot/Push_30KB_snapshot - B/op 7808286 B/op 7751132 B/op 1.01
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op 81801 allocs/op 80497 allocs/op 1.02
BenchmarkSnapshot/Pull_3KB_snapshot 7355077 ns/op 1141507 B/op 19419 allocs/op 7383662 ns/op 1141810 B/op 19419 allocs/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op 7355077 ns/op 7383662 ns/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot - B/op 1141507 B/op 1141810 B/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op 19419 allocs/op 19419 allocs/op 1
BenchmarkSnapshot/Pull_30KB_snapshot 18763306 ns/op 9324394 B/op 187580 allocs/op 18934986 ns/op 9322258 B/op 187579 allocs/op 0.99
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op 18763306 ns/op 18934986 ns/op 0.99
BenchmarkSnapshot/Pull_30KB_snapshot - B/op 9324394 B/op 9322258 B/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op 187580 allocs/op 187579 allocs/op 1.00
BenchmarkSplayTree/stress_test_100000 0.203 ns/op 0 B/op 0 allocs/op 0.2001 ns/op 0 B/op 0 allocs/op 1.01
BenchmarkSplayTree/stress_test_100000 - ns/op 0.203 ns/op 0.2001 ns/op 1.01
BenchmarkSplayTree/stress_test_100000 - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/stress_test_100000 - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSplayTree/stress_test_200000 0.3941 ns/op 0 B/op 0 allocs/op 0.4127 ns/op 0 B/op 0 allocs/op 0.95
BenchmarkSplayTree/stress_test_200000 - ns/op 0.3941 ns/op 0.4127 ns/op 0.95
BenchmarkSplayTree/stress_test_200000 - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/stress_test_200000 - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSplayTree/stress_test_300000 0.6087 ns/op 0 B/op 0 allocs/op 0.5901 ns/op 0 B/op 0 allocs/op 1.03
BenchmarkSplayTree/stress_test_300000 - ns/op 0.6087 ns/op 0.5901 ns/op 1.03
BenchmarkSplayTree/stress_test_300000 - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/stress_test_300000 - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSplayTree/random_access_100000 0.01417 ns/op 0 B/op 0 allocs/op 0.01282 ns/op 0 B/op 0 allocs/op 1.11
BenchmarkSplayTree/random_access_100000 - ns/op 0.01417 ns/op 0.01282 ns/op 1.11
BenchmarkSplayTree/random_access_100000 - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/random_access_100000 - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSplayTree/random_access_200000 0.03358 ns/op 0 B/op 0 allocs/op 0.02991 ns/op 0 B/op 0 allocs/op 1.12
BenchmarkSplayTree/random_access_200000 - ns/op 0.03358 ns/op 0.02991 ns/op 1.12
BenchmarkSplayTree/random_access_200000 - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/random_access_200000 - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSplayTree/random_access_300000 0.04421 ns/op 0 B/op 0 allocs/op 0.04209 ns/op 0 B/op 0 allocs/op 1.05
BenchmarkSplayTree/random_access_300000 - ns/op 0.04421 ns/op 0.04209 ns/op 1.05
BenchmarkSplayTree/random_access_300000 - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/random_access_300000 - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSplayTree/editing_trace_bench 0.001688 ns/op 0 B/op 0 allocs/op 0.001983 ns/op 0 B/op 0 allocs/op 0.85
BenchmarkSplayTree/editing_trace_bench - ns/op 0.001688 ns/op 0.001983 ns/op 0.85
BenchmarkSplayTree/editing_trace_bench - B/op 0 B/op 0 B/op 1
BenchmarkSplayTree/editing_trace_bench - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkSync/memory_sync_10_test 8950 ns/op 3765 B/op 69 allocs/op 8526 ns/op 3765 B/op 69 allocs/op 1.05
BenchmarkSync/memory_sync_10_test - ns/op 8950 ns/op 8526 ns/op 1.05
BenchmarkSync/memory_sync_10_test - B/op 3765 B/op 3765 B/op 1
BenchmarkSync/memory_sync_10_test - allocs/op 69 allocs/op 69 allocs/op 1
BenchmarkSync/memory_sync_100_test 55041 ns/op 11108 B/op 303 allocs/op 55076 ns/op 11114 B/op 303 allocs/op 1.00
BenchmarkSync/memory_sync_100_test - ns/op 55041 ns/op 55076 ns/op 1.00
BenchmarkSync/memory_sync_100_test - B/op 11108 B/op 11114 B/op 1.00
BenchmarkSync/memory_sync_100_test - allocs/op 303 allocs/op 303 allocs/op 1
BenchmarkSync/memory_sync_1000_test 591910 ns/op 77039 B/op 2150 allocs/op 583580 ns/op 77244 B/op 2165 allocs/op 1.01
BenchmarkSync/memory_sync_1000_test - ns/op 591910 ns/op 583580 ns/op 1.01
BenchmarkSync/memory_sync_1000_test - B/op 77039 B/op 77244 B/op 1.00
BenchmarkSync/memory_sync_1000_test - allocs/op 2150 allocs/op 2165 allocs/op 0.99
BenchmarkSync/memory_sync_10000_test 7300789 ns/op 768652 B/op 20668 allocs/op 7492576 ns/op 759406 B/op 20502 allocs/op 0.97
BenchmarkSync/memory_sync_10000_test - ns/op 7300789 ns/op 7492576 ns/op 0.97
BenchmarkSync/memory_sync_10000_test - B/op 768652 B/op 759406 B/op 1.01
BenchmarkSync/memory_sync_10000_test - allocs/op 20668 allocs/op 20502 allocs/op 1.01
BenchmarkTextEditing 4883169997 ns/op 3982598448 B/op 20647678 allocs/op 5069821521 ns/op 3982601376 B/op 20647693 allocs/op 0.96
BenchmarkTextEditing - ns/op 4883169997 ns/op 5069821521 ns/op 0.96
BenchmarkTextEditing - B/op 3982598448 B/op 3982601376 B/op 1.00
BenchmarkTextEditing - allocs/op 20647678 allocs/op 20647693 allocs/op 1.00
BenchmarkTree/10000_vertices_to_protobuf 4053098 ns/op 6263012 B/op 70025 allocs/op 4195320 ns/op 6263041 B/op 70025 allocs/op 0.97
BenchmarkTree/10000_vertices_to_protobuf - ns/op 4053098 ns/op 4195320 ns/op 0.97
BenchmarkTree/10000_vertices_to_protobuf - B/op 6263012 B/op 6263041 B/op 1.00
BenchmarkTree/10000_vertices_to_protobuf - allocs/op 70025 allocs/op 70025 allocs/op 1
BenchmarkTree/10000_vertices_from_protobuf 222513428 ns/op 442173726 B/op 290040 allocs/op 214088359 ns/op 442170308 B/op 290039 allocs/op 1.04
BenchmarkTree/10000_vertices_from_protobuf - ns/op 222513428 ns/op 214088359 ns/op 1.04
BenchmarkTree/10000_vertices_from_protobuf - B/op 442173726 B/op 442170308 B/op 1.00
BenchmarkTree/10000_vertices_from_protobuf - allocs/op 290040 allocs/op 290039 allocs/op 1.00
BenchmarkTree/20000_vertices_to_protobuf 8876775 ns/op 12721784 B/op 140028 allocs/op 8508506 ns/op 12721534 B/op 140028 allocs/op 1.04
BenchmarkTree/20000_vertices_to_protobuf - ns/op 8876775 ns/op 8508506 ns/op 1.04
BenchmarkTree/20000_vertices_to_protobuf - B/op 12721784 B/op 12721534 B/op 1.00
BenchmarkTree/20000_vertices_to_protobuf - allocs/op 140028 allocs/op 140028 allocs/op 1
BenchmarkTree/20000_vertices_from_protobuf 838145207 ns/op 1697271768 B/op 580042 allocs/op 842228831 ns/op 1697267672 B/op 580041 allocs/op 1.00
BenchmarkTree/20000_vertices_from_protobuf - ns/op 838145207 ns/op 842228831 ns/op 1.00
BenchmarkTree/20000_vertices_from_protobuf - B/op 1697271768 B/op 1697267672 B/op 1.00
BenchmarkTree/20000_vertices_from_protobuf - allocs/op 580042 allocs/op 580041 allocs/op 1.00
BenchmarkTree/30000_vertices_to_protobuf 14019898 ns/op 19318245 B/op 210030 allocs/op 14003700 ns/op 19318351 B/op 210030 allocs/op 1.00
BenchmarkTree/30000_vertices_to_protobuf - ns/op 14019898 ns/op 14003700 ns/op 1.00
BenchmarkTree/30000_vertices_to_protobuf - B/op 19318245 B/op 19318351 B/op 1.00
BenchmarkTree/30000_vertices_to_protobuf - allocs/op 210030 allocs/op 210030 allocs/op 1
BenchmarkTree/30000_vertices_from_protobuf 1917049672 ns/op 3752061016 B/op 870137 allocs/op 1925258613 ns/op 3752061016 B/op 870137 allocs/op 1.00
BenchmarkTree/30000_vertices_from_protobuf - ns/op 1917049672 ns/op 1925258613 ns/op 1.00
BenchmarkTree/30000_vertices_from_protobuf - B/op 3752061016 B/op 3752061016 B/op 1
BenchmarkTree/30000_vertices_from_protobuf - allocs/op 870137 allocs/op 870137 allocs/op 1

This comment was automatically generated by workflow using github-action-benchmark.

@raararaara raararaara changed the title Fix deletion of the version vector when client deactivation Fix version vector not being removed correctly during document detach on client deactivate Nov 19, 2024
@raararaara raararaara marked this pull request as ready for review November 19, 2024 07:22
coderabbitai[bot]
coderabbitai bot reviewed Nov 19, 2024
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
test/integration/gc_test.go (1)

1145-1181: Improve assertion readability and consistency

The assertion style can be made more consistent and readable throughout the test.

Consider this improvement:

-assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 1)))
+assert.True(t, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 1)))

This change:

  • Uses assert.True() which is more idiomatic for boolean assertions
  • Makes the intent clearer
  • Maintains consistency with other test files
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f8ebba2 and e18af1e.

📒 Files selected for processing (2)
  • server/clients/clients.go (0 hunks)
  • test/integration/gc_test.go (2 hunks)
💤 Files with no reviewable changes (1)
  • server/clients/clients.go
🔇 Additional comments (1)
test/integration/gc_test.go (1)

1190-1195: LGTM! Good test coverage for client deactivation

The new assertions effectively verify two critical aspects:

  1. Garbage collection behavior after client deactivation
  2. Version vector cleanup post-deactivation

The test ensures that:

  • Garbage is properly collected (d2.GarbageLen() changes from 2 to 0)
  • Version vector is correctly updated (length reduces from 2 to 1)

This provides good coverage for the fix described in the PR objectives regarding version vector cleanup during client deactivation.

@hackerwins hackerwins changed the title Fix version vector not being removed correctly during document detach on client deactivate Fix version vector cleanup during client deactivation Nov 19, 2024
hackerwins
hackerwins approved these changes Nov 19, 2024
View reviewed changes
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution.

@hackerwins hackerwins merged commit e629fd0 into main Nov 19, 2024
5 checks passed
@hackerwins hackerwins deleted the deactivate-vv-deletion branch November 19, 2024 08:50
@coderabbitai coderabbitai bot mentioned this pull request Nov 20, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@hackerwins hackerwins hackerwins approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deletion of the version vector of a deactivated client is not performed properly
2 participants
@raararaara @hackerwins