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 transaction in UpdateAndFindMinSyncedVersionVector #1050

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Fix transaction in UpdateAndFindMinSyncedVersionVector #1050

merged 1 commit into from
Oct 25, 2024

Conversation

hackerwins
Copy link
Member

@hackerwins hackerwins commented Oct 25, 2024 •
edited
Loading

What this PR does / why we need it:

Fix transaction in UpdateAndFindMinSyncedVersionVector

This commit resolves an issue where the PushPull operation does not function correctly when using the Memory DB. UpdateAndFindMinSyncedVersionVector was previously set to start in write mode, which causes it to simply read documents. However, inside this function, it calls UpdateVersionVector, which attempts to start in write mode again, resulting in a deadlock scenario where the transaction cannot be acquired, causing an infinite wait.

Which issue(s) this PR fixes:

Fixes #

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

  • New Features

    • Enhanced transaction handling for fetching version vectors, improving database interaction.
    • Updated logic to compute the minimum version vector while filtering out detached clients.

  • Bug Fixes

    • Refined method to ensure accurate computation of the minimum synced version vector after push-pull operations.

  • Documentation

    • Minor adjustments made to comments for improved clarity.

@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2024 •
edited
Loading

Walkthrough

The changes involve renaming the UpdateAndFindMinSyncedVersionVectorAfterPushPull method to UpdateAndFindMinSyncedVersionVector across several files, including the DB struct in server/backend/database/memory/database.go, the Database interface in server/backend/database/database.go, and the Client struct in server/backend/database/mongo/client.go. The transaction mode for the method in the DB struct has been switched from write to read, and the logic for computing the minimum version vector has been refined to filter out detached clients. Minor comment adjustments were also made for clarity.

Changes

File Path Change Summary
server/backend/database/memory/database.go Renamed method from UpdateAndFindMinSyncedVersionVectorAfterPushPull to UpdateAndFindMinSyncedVersionVector; changed transaction mode from write to read; refined logic to compute minimum version vector while filtering detached clients; minor comment adjustments.
server/backend/database/database.go Renamed method from UpdateAndFindMinSyncedVersionVectorAfterPushPull to UpdateAndFindMinSyncedVersionVector; updated comments for clarity.
server/backend/database/mongo/client.go Renamed method from UpdateAndFindMinSyncedVersionVectorAfterPushPull to UpdateAndFindMinSyncedVersionVector; adjusted comments for clarity.
server/packs/packs.go Updated method call from UpdateAndFindMinSyncedVersionVectorAfterPushPull to UpdateAndFindMinSyncedVersionVector in PushPull function.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant DB
    participant VersionVector

    Client->>DB: Request UpdateAndFindMinSyncedVersionVector
    DB->>DB: Start transaction (read mode)
    DB->>VersionVector: Fetch attached clients' version vectors
    DB->>DB: Compute minimum version vector
    DB-->>Client: Return minimum version vector
Loading

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 Oct 25, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 46.44%. Comparing base (bd2ebe9) to head (aea5cd9).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/backend/database/memory/database.go 0.00% 1 Missing ⚠️
server/packs/packs.go 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1050   +/-   ##
=======================================
  Coverage   46.44%   46.44%           
=======================================
  Files          81       81           
  Lines       11887    11887           
=======================================
  Hits         5521     5521           
  Misses       5806     5806           
  Partials      560      560

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

@hackerwins hackerwins changed the title Fix transaction in UpdateAndFindMinSyncedVV Fix transaction in UpdateAndFindMinSyncedVersionVector Oct 25, 2024
github-actions[bot]
github-actions bot reviewed Oct 25, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

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: aea5cd9 Previous: 1b88ea1 Ratio
BenchmarkDocument/constructor_test 1498 ns/op 1337 B/op 24 allocs/op 1473 ns/op 1337 B/op 24 allocs/op 1.02
BenchmarkDocument/constructor_test - ns/op 1498 ns/op 1473 ns/op 1.02
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 972.3 ns/op 1305 B/op 22 allocs/op 927.3 ns/op 1305 B/op 22 allocs/op 1.05
BenchmarkDocument/status_test - ns/op 972.3 ns/op 927.3 ns/op 1.05
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 7799 ns/op 7529 B/op 134 allocs/op 7751 ns/op 7529 B/op 134 allocs/op 1.01
BenchmarkDocument/equals_test - ns/op 7799 ns/op 7751 ns/op 1.01
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 16967 ns/op 12395 B/op 264 allocs/op 16915 ns/op 12395 B/op 264 allocs/op 1.00
BenchmarkDocument/nested_update_test - ns/op 16967 ns/op 16915 ns/op 1.00
BenchmarkDocument/nested_update_test - B/op 12395 B/op 12395 B/op 1
BenchmarkDocument/nested_update_test - allocs/op 264 allocs/op 264 allocs/op 1
BenchmarkDocument/delete_test 23769 ns/op 15923 B/op 347 allocs/op 25566 ns/op 15923 B/op 347 allocs/op 0.93
BenchmarkDocument/delete_test - ns/op 23769 ns/op 25566 ns/op 0.93
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 9656 ns/op 7073 B/op 122 allocs/op 8926 ns/op 7073 B/op 122 allocs/op 1.08
BenchmarkDocument/object_test - ns/op 9656 ns/op 8926 ns/op 1.08
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 30974 ns/op 12203 B/op 278 allocs/op 29694 ns/op 12203 B/op 278 allocs/op 1.04
BenchmarkDocument/array_test - ns/op 30974 ns/op 29694 ns/op 1.04
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 32073 ns/op 15324 B/op 492 allocs/op 32421 ns/op 15324 B/op 492 allocs/op 0.99
BenchmarkDocument/text_test - ns/op 32073 ns/op 32421 ns/op 0.99
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 31057 ns/op 18718 B/op 504 allocs/op 31304 ns/op 18719 B/op 504 allocs/op 0.99
BenchmarkDocument/text_composition_test - ns/op 31057 ns/op 31304 ns/op 0.99
BenchmarkDocument/text_composition_test - B/op 18718 B/op 18719 B/op 1.00
BenchmarkDocument/text_composition_test - allocs/op 504 allocs/op 504 allocs/op 1
BenchmarkDocument/rich_text_test 84101 ns/op 40180 B/op 1183 allocs/op 83647 ns/op 40181 B/op 1183 allocs/op 1.01
BenchmarkDocument/rich_text_test - ns/op 84101 ns/op 83647 ns/op 1.01
BenchmarkDocument/rich_text_test - B/op 40180 B/op 40181 B/op 1.00
BenchmarkDocument/rich_text_test - allocs/op 1183 allocs/op 1183 allocs/op 1
BenchmarkDocument/counter_test 18556 ns/op 11874 B/op 258 allocs/op 18631 ns/op 11874 B/op 258 allocs/op 1.00
BenchmarkDocument/counter_test - ns/op 18556 ns/op 18631 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 1320866 ns/op 872578 B/op 17281 allocs/op 1314315 ns/op 872525 B/op 17281 allocs/op 1.00
BenchmarkDocument/text_edit_gc_100 - ns/op 1320866 ns/op 1314315 ns/op 1.00
BenchmarkDocument/text_edit_gc_100 - B/op 872578 B/op 872525 B/op 1.00
BenchmarkDocument/text_edit_gc_100 - allocs/op 17281 allocs/op 17281 allocs/op 1
BenchmarkDocument/text_edit_gc_1000 50888289 ns/op 50546202 B/op 186739 allocs/op 49874900 ns/op 50546639 B/op 186736 allocs/op 1.02
BenchmarkDocument/text_edit_gc_1000 - ns/op 50888289 ns/op 49874900 ns/op 1.02
BenchmarkDocument/text_edit_gc_1000 - B/op 50546202 B/op 50546639 B/op 1.00
BenchmarkDocument/text_edit_gc_1000 - allocs/op 186739 allocs/op 186736 allocs/op 1.00
BenchmarkDocument/text_split_gc_100 1938048 ns/op 1589048 B/op 15950 allocs/op 1922820 ns/op 1589081 B/op 15951 allocs/op 1.01
BenchmarkDocument/text_split_gc_100 - ns/op 1938048 ns/op 1922820 ns/op 1.01
BenchmarkDocument/text_split_gc_100 - B/op 1589048 B/op 1589081 B/op 1.00
BenchmarkDocument/text_split_gc_100 - allocs/op 15950 allocs/op 15951 allocs/op 1.00
BenchmarkDocument/text_split_gc_1000 117427747 ns/op 141482164 B/op 186146 allocs/op 115105979 ns/op 141481604 B/op 186137 allocs/op 1.02
BenchmarkDocument/text_split_gc_1000 - ns/op 117427747 ns/op 115105979 ns/op 1.02
BenchmarkDocument/text_split_gc_1000 - B/op 141482164 B/op 141481604 B/op 1.00
BenchmarkDocument/text_split_gc_1000 - allocs/op 186146 allocs/op 186137 allocs/op 1.00
BenchmarkDocument/text_delete_all_10000 16930448 ns/op 10214574 B/op 55688 allocs/op 16376388 ns/op 10214032 B/op 55685 allocs/op 1.03
BenchmarkDocument/text_delete_all_10000 - ns/op 16930448 ns/op 16376388 ns/op 1.03
BenchmarkDocument/text_delete_all_10000 - B/op 10214574 B/op 10214032 B/op 1.00
BenchmarkDocument/text_delete_all_10000 - allocs/op 55688 allocs/op 55685 allocs/op 1.00
BenchmarkDocument/text_delete_all_100000 304485900 ns/op 142976140 B/op 561701 allocs/op 285180946 ns/op 142972684 B/op 561704 allocs/op 1.07
BenchmarkDocument/text_delete_all_100000 - ns/op 304485900 ns/op 285180946 ns/op 1.07
BenchmarkDocument/text_delete_all_100000 - B/op 142976140 B/op 142972684 B/op 1.00
BenchmarkDocument/text_delete_all_100000 - allocs/op 561701 allocs/op 561704 allocs/op 1.00
BenchmarkDocument/text_100 230317 ns/op 120489 B/op 5182 allocs/op 217265 ns/op 120491 B/op 5182 allocs/op 1.06
BenchmarkDocument/text_100 - ns/op 230317 ns/op 217265 ns/op 1.06
BenchmarkDocument/text_100 - B/op 120489 B/op 120491 B/op 1.00
BenchmarkDocument/text_100 - allocs/op 5182 allocs/op 5182 allocs/op 1
BenchmarkDocument/text_1000 2468371 ns/op 1171279 B/op 51086 allocs/op 2369607 ns/op 1171277 B/op 51086 allocs/op 1.04
BenchmarkDocument/text_1000 - ns/op 2468371 ns/op 2369607 ns/op 1.04
BenchmarkDocument/text_1000 - B/op 1171279 B/op 1171277 B/op 1.00
BenchmarkDocument/text_1000 - allocs/op 51086 allocs/op 51086 allocs/op 1
BenchmarkDocument/array_1000 1282623 ns/op 1091630 B/op 11833 allocs/op 1212984 ns/op 1091664 B/op 11834 allocs/op 1.06
BenchmarkDocument/array_1000 - ns/op 1282623 ns/op 1212984 ns/op 1.06
BenchmarkDocument/array_1000 - B/op 1091630 B/op 1091664 B/op 1.00
BenchmarkDocument/array_1000 - allocs/op 11833 allocs/op 11834 allocs/op 1.00
BenchmarkDocument/array_10000 13439414 ns/op 9800126 B/op 120298 allocs/op 13143551 ns/op 9800231 B/op 120298 allocs/op 1.02
BenchmarkDocument/array_10000 - ns/op 13439414 ns/op 13143551 ns/op 1.02
BenchmarkDocument/array_10000 - B/op 9800126 B/op 9800231 B/op 1.00
BenchmarkDocument/array_10000 - allocs/op 120298 allocs/op 120298 allocs/op 1
BenchmarkDocument/array_gc_100 156317 ns/op 133277 B/op 1266 allocs/op 146585 ns/op 133290 B/op 1267 allocs/op 1.07
BenchmarkDocument/array_gc_100 - ns/op 156317 ns/op 146585 ns/op 1.07
BenchmarkDocument/array_gc_100 - B/op 133277 B/op 133290 B/op 1.00
BenchmarkDocument/array_gc_100 - allocs/op 1266 allocs/op 1267 allocs/op 1.00
BenchmarkDocument/array_gc_1000 1458716 ns/op 1159635 B/op 12882 allocs/op 1386398 ns/op 1159824 B/op 12883 allocs/op 1.05
BenchmarkDocument/array_gc_1000 - ns/op 1458716 ns/op 1386398 ns/op 1.05
BenchmarkDocument/array_gc_1000 - B/op 1159635 B/op 1159824 B/op 1.00
BenchmarkDocument/array_gc_1000 - allocs/op 12882 allocs/op 12883 allocs/op 1.00
BenchmarkDocument/counter_1000 216341 ns/op 193337 B/op 5773 allocs/op 199672 ns/op 193336 B/op 5773 allocs/op 1.08
BenchmarkDocument/counter_1000 - ns/op 216341 ns/op 199672 ns/op 1.08
BenchmarkDocument/counter_1000 - B/op 193337 B/op 193336 B/op 1.00
BenchmarkDocument/counter_1000 - allocs/op 5773 allocs/op 5773 allocs/op 1
BenchmarkDocument/counter_10000 2270223 ns/op 2088255 B/op 59780 allocs/op 2161187 ns/op 2088267 B/op 59780 allocs/op 1.05
BenchmarkDocument/counter_10000 - ns/op 2270223 ns/op 2161187 ns/op 1.05
BenchmarkDocument/counter_10000 - B/op 2088255 B/op 2088267 B/op 1.00
BenchmarkDocument/counter_10000 - allocs/op 59780 allocs/op 59780 allocs/op 1
BenchmarkDocument/object_1000 1462648 ns/op 1428434 B/op 9851 allocs/op 1361897 ns/op 1428281 B/op 9850 allocs/op 1.07
BenchmarkDocument/object_1000 - ns/op 1462648 ns/op 1361897 ns/op 1.07
BenchmarkDocument/object_1000 - B/op 1428434 B/op 1428281 B/op 1.00
BenchmarkDocument/object_1000 - allocs/op 9851 allocs/op 9850 allocs/op 1.00
BenchmarkDocument/object_10000 15928111 ns/op 12166907 B/op 100566 allocs/op 15066992 ns/op 12167973 B/op 100568 allocs/op 1.06
BenchmarkDocument/object_10000 - ns/op 15928111 ns/op 15066992 ns/op 1.06
BenchmarkDocument/object_10000 - B/op 12166907 B/op 12167973 B/op 1.00
BenchmarkDocument/object_10000 - allocs/op 100566 allocs/op 100568 allocs/op 1.00
BenchmarkDocument/tree_100 1078657 ns/op 943958 B/op 6103 allocs/op 1012966 ns/op 943963 B/op 6103 allocs/op 1.06
BenchmarkDocument/tree_100 - ns/op 1078657 ns/op 1012966 ns/op 1.06
BenchmarkDocument/tree_100 - B/op 943958 B/op 943963 B/op 1.00
BenchmarkDocument/tree_100 - allocs/op 6103 allocs/op 6103 allocs/op 1
BenchmarkDocument/tree_1000 79477204 ns/op 86460796 B/op 60116 allocs/op 70462610 ns/op 86460362 B/op 60116 allocs/op 1.13
BenchmarkDocument/tree_1000 - ns/op 79477204 ns/op 70462610 ns/op 1.13
BenchmarkDocument/tree_1000 - B/op 86460796 B/op 86460362 B/op 1.00
BenchmarkDocument/tree_1000 - allocs/op 60116 allocs/op 60116 allocs/op 1
BenchmarkDocument/tree_10000 9827763816 ns/op 8580669200 B/op 600222 allocs/op 9069718722 ns/op 8580671648 B/op 600237 allocs/op 1.08
BenchmarkDocument/tree_10000 - ns/op 9827763816 ns/op 9069718722 ns/op 1.08
BenchmarkDocument/tree_10000 - B/op 8580669200 B/op 8580671648 B/op 1.00
BenchmarkDocument/tree_10000 - allocs/op 600222 allocs/op 600237 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 80145387 ns/op 87510384 B/op 75272 allocs/op 72195496 ns/op 87511267 B/op 75273 allocs/op 1.11
BenchmarkDocument/tree_delete_all_1000 - ns/op 80145387 ns/op 72195496 ns/op 1.11
BenchmarkDocument/tree_delete_all_1000 - B/op 87510384 B/op 87511267 B/op 1.00
BenchmarkDocument/tree_delete_all_1000 - allocs/op 75272 allocs/op 75273 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_100 4020159 ns/op 4147452 B/op 15147 allocs/op 3684967 ns/op 4148199 B/op 15146 allocs/op 1.09
BenchmarkDocument/tree_edit_gc_100 - ns/op 4020159 ns/op 3684967 ns/op 1.09
BenchmarkDocument/tree_edit_gc_100 - B/op 4147452 B/op 4148199 B/op 1.00
BenchmarkDocument/tree_edit_gc_100 - allocs/op 15147 allocs/op 15146 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_1000 323397489 ns/op 383749138 B/op 154877 allocs/op 292347190 ns/op 383744968 B/op 154856 allocs/op 1.11
BenchmarkDocument/tree_edit_gc_1000 - ns/op 323397489 ns/op 292347190 ns/op 1.11
BenchmarkDocument/tree_edit_gc_1000 - B/op 383749138 B/op 383744968 B/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - allocs/op 154877 allocs/op 154856 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 2703855 ns/op 2413135 B/op 11131 allocs/op 2471274 ns/op 2413065 B/op 11131 allocs/op 1.09
BenchmarkDocument/tree_split_gc_100 - ns/op 2703855 ns/op 2471274 ns/op 1.09
BenchmarkDocument/tree_split_gc_100 - B/op 2413135 B/op 2413065 B/op 1.00
BenchmarkDocument/tree_split_gc_100 - allocs/op 11131 allocs/op 11131 allocs/op 1
BenchmarkDocument/tree_split_gc_1000 196487176 ns/op 222256064 B/op 122016 allocs/op 177465670 ns/op 222253045 B/op 121996 allocs/op 1.11
BenchmarkDocument/tree_split_gc_1000 - ns/op 196487176 ns/op 177465670 ns/op 1.11
BenchmarkDocument/tree_split_gc_1000 - B/op 222256064 B/op 222253045 B/op 1.00
BenchmarkDocument/tree_split_gc_1000 - allocs/op 122016 allocs/op 121996 allocs/op 1.00
BenchmarkRPC/client_to_server 423889335 ns/op 21378752 B/op 230355 allocs/op 419281295 ns/op 20278080 B/op 230305 allocs/op 1.01
BenchmarkRPC/client_to_server - ns/op 423889335 ns/op 419281295 ns/op 1.01
BenchmarkRPC/client_to_server - B/op 21378752 B/op 20278080 B/op 1.05
BenchmarkRPC/client_to_server - allocs/op 230355 allocs/op 230305 allocs/op 1.00
BenchmarkRPC/client_to_client_via_server 789444886 ns/op 41140556 B/op 484267 allocs/op 777268560 ns/op 41929792 B/op 484230 allocs/op 1.02
BenchmarkRPC/client_to_client_via_server - ns/op 789444886 ns/op 777268560 ns/op 1.02
BenchmarkRPC/client_to_client_via_server - B/op 41140556 B/op 41929792 B/op 0.98
BenchmarkRPC/client_to_client_via_server - allocs/op 484267 allocs/op 484230 allocs/op 1.00
BenchmarkRPC/attach_large_document 2076356942 ns/op 3023205880 B/op 14943 allocs/op 2153660169 ns/op 3022421544 B/op 15087 allocs/op 0.96
BenchmarkRPC/attach_large_document - ns/op 2076356942 ns/op 2153660169 ns/op 0.96
BenchmarkRPC/attach_large_document - B/op 3023205880 B/op 3022421544 B/op 1.00
BenchmarkRPC/attach_large_document - allocs/op 14943 allocs/op 15087 allocs/op 0.99
BenchmarkRPC/adminCli_to_server 533149792 ns/op 35968348 B/op 289566 allocs/op 521274065 ns/op 35954232 B/op 289576 allocs/op 1.02
BenchmarkRPC/adminCli_to_server - ns/op 533149792 ns/op 521274065 ns/op 1.02
BenchmarkRPC/adminCli_to_server - B/op 35968348 B/op 35954232 B/op 1.00
BenchmarkRPC/adminCli_to_server - allocs/op 289566 allocs/op 289576 allocs/op 1.00
BenchmarkLocker 64.87 ns/op 16 B/op 1 allocs/op 63.95 ns/op 16 B/op 1 allocs/op 1.01
BenchmarkLocker - ns/op 64.87 ns/op 63.95 ns/op 1.01
BenchmarkLocker - B/op 16 B/op 16 B/op 1
BenchmarkLocker - allocs/op 1 allocs/op 1 allocs/op 1
BenchmarkLockerParallel 39.1 ns/op 0 B/op 0 allocs/op 38.88 ns/op 0 B/op 0 allocs/op 1.01
BenchmarkLockerParallel - ns/op 39.1 ns/op 38.88 ns/op 1.01
BenchmarkLockerParallel - B/op 0 B/op 0 B/op 1
BenchmarkLockerParallel - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkLockerMoreKeys 156.2 ns/op 15 B/op 0 allocs/op 159.7 ns/op 15 B/op 0 allocs/op 0.98
BenchmarkLockerMoreKeys - ns/op 156.2 ns/op 159.7 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 4391207 ns/op 143386 B/op 1572 allocs/op 4391875 ns/op 143615 B/op 1572 allocs/op 1.00
BenchmarkChange/Push_10_Changes - ns/op 4391207 ns/op 4391875 ns/op 1.00
BenchmarkChange/Push_10_Changes - B/op 143386 B/op 143615 B/op 1.00
BenchmarkChange/Push_10_Changes - allocs/op 1572 allocs/op 1572 allocs/op 1
BenchmarkChange/Push_100_Changes 15936227 ns/op 700790 B/op 8188 allocs/op 15975847 ns/op 696367 B/op 8188 allocs/op 1.00
BenchmarkChange/Push_100_Changes - ns/op 15936227 ns/op 15975847 ns/op 1.00
BenchmarkChange/Push_100_Changes - B/op 700790 B/op 696367 B/op 1.01
BenchmarkChange/Push_100_Changes - allocs/op 8188 allocs/op 8188 allocs/op 1
BenchmarkChange/Push_1000_Changes 127414430 ns/op 6785030 B/op 77162 allocs/op 129197187 ns/op 6709595 B/op 77159 allocs/op 0.99
BenchmarkChange/Push_1000_Changes - ns/op 127414430 ns/op 129197187 ns/op 0.99
BenchmarkChange/Push_1000_Changes - B/op 6785030 B/op 6709595 B/op 1.01
BenchmarkChange/Push_1000_Changes - allocs/op 77162 allocs/op 77159 allocs/op 1.00
BenchmarkChange/Pull_10_Changes 3680473 ns/op 123969 B/op 1405 allocs/op 3582745 ns/op 124271 B/op 1405 allocs/op 1.03
BenchmarkChange/Pull_10_Changes - ns/op 3680473 ns/op 3582745 ns/op 1.03
BenchmarkChange/Pull_10_Changes - B/op 123969 B/op 124271 B/op 1.00
BenchmarkChange/Pull_10_Changes - allocs/op 1405 allocs/op 1405 allocs/op 1
BenchmarkChange/Pull_100_Changes 5283259 ns/op 350920 B/op 4948 allocs/op 5072397 ns/op 352400 B/op 4949 allocs/op 1.04
BenchmarkChange/Pull_100_Changes - ns/op 5283259 ns/op 5072397 ns/op 1.04
BenchmarkChange/Pull_100_Changes - B/op 350920 B/op 352400 B/op 1.00
BenchmarkChange/Pull_100_Changes - allocs/op 4948 allocs/op 4949 allocs/op 1.00
BenchmarkChange/Pull_1000_Changes 10373938 ns/op 2220374 B/op 42667 allocs/op 10005059 ns/op 2221661 B/op 42667 allocs/op 1.04
BenchmarkChange/Pull_1000_Changes - ns/op 10373938 ns/op 10005059 ns/op 1.04
BenchmarkChange/Pull_1000_Changes - B/op 2220374 B/op 2221661 B/op 1.00
BenchmarkChange/Pull_1000_Changes - allocs/op 42667 allocs/op 42667 allocs/op 1
BenchmarkSnapshot/Push_3KB_snapshot 18080312 ns/op 815158 B/op 8191 allocs/op 18342089 ns/op 812502 B/op 8190 allocs/op 0.99
BenchmarkSnapshot/Push_3KB_snapshot - ns/op 18080312 ns/op 18342089 ns/op 0.99
BenchmarkSnapshot/Push_3KB_snapshot - B/op 815158 B/op 812502 B/op 1.00
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op 8191 allocs/op 8190 allocs/op 1.00
BenchmarkSnapshot/Push_30KB_snapshot 128032293 ns/op 7131498 B/op 77766 allocs/op 131237301 ns/op 7127180 B/op 77894 allocs/op 0.98
BenchmarkSnapshot/Push_30KB_snapshot - ns/op 128032293 ns/op 131237301 ns/op 0.98
BenchmarkSnapshot/Push_30KB_snapshot - B/op 7131498 B/op 7127180 B/op 1.00
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op 77766 allocs/op 77894 allocs/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot 7234164 ns/op 1137490 B/op 19416 allocs/op 7028336 ns/op 1139816 B/op 19418 allocs/op 1.03
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op 7234164 ns/op 7028336 ns/op 1.03
BenchmarkSnapshot/Pull_3KB_snapshot - B/op 1137490 B/op 1139816 B/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op 19416 allocs/op 19418 allocs/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot 17592126 ns/op 9295182 B/op 187557 allocs/op 17091343 ns/op 9289206 B/op 187495 allocs/op 1.03
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op 17592126 ns/op 17091343 ns/op 1.03
BenchmarkSnapshot/Pull_30KB_snapshot - B/op 9295182 B/op 9289206 B/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op 187557 allocs/op 187495 allocs/op 1.00
BenchmarkSplayTree/stress_test_100000 0.1953 ns/op 0 B/op 0 allocs/op 0.1902 ns/op 0 B/op 0 allocs/op 1.03
BenchmarkSplayTree/stress_test_100000 - ns/op 0.1953 ns/op 0.1902 ns/op 1.03
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.3873 ns/op 0 B/op 0 allocs/op 0.3909 ns/op 0 B/op 0 allocs/op 0.99
BenchmarkSplayTree/stress_test_200000 - ns/op 0.3873 ns/op 0.3909 ns/op 0.99
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.6 ns/op 0 B/op 0 allocs/op 0.5799 ns/op 0 B/op 0 allocs/op 1.03
BenchmarkSplayTree/stress_test_300000 - ns/op 0.6 ns/op 0.5799 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.01264 ns/op 0 B/op 0 allocs/op 0.01309 ns/op 0 B/op 0 allocs/op 0.97
BenchmarkSplayTree/random_access_100000 - ns/op 0.01264 ns/op 0.01309 ns/op 0.97
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.02448 ns/op 0 B/op 0 allocs/op 0.02362 ns/op 0 B/op 0 allocs/op 1.04
BenchmarkSplayTree/random_access_200000 - ns/op 0.02448 ns/op 0.02362 ns/op 1.04
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.03903 ns/op 0 B/op 0 allocs/op 0.03521 ns/op 0 B/op 0 allocs/op 1.11
BenchmarkSplayTree/random_access_300000 - ns/op 0.03903 ns/op 0.03521 ns/op 1.11
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.002523 ns/op 0 B/op 0 allocs/op 0.001788 ns/op 0 B/op 0 allocs/op 1.41
BenchmarkSplayTree/editing_trace_bench - ns/op 0.002523 ns/op 0.001788 ns/op 1.41
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 6820 ns/op 1286 B/op 38 allocs/op 6795 ns/op 1286 B/op 38 allocs/op 1.00
BenchmarkSync/memory_sync_10_test - ns/op 6820 ns/op 6795 ns/op 1.00
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 51401 ns/op 8643 B/op 273 allocs/op 51041 ns/op 8634 B/op 272 allocs/op 1.01
BenchmarkSync/memory_sync_100_test - ns/op 51401 ns/op 51041 ns/op 1.01
BenchmarkSync/memory_sync_100_test - B/op 8643 B/op 8634 B/op 1.00
BenchmarkSync/memory_sync_100_test - allocs/op 273 allocs/op 272 allocs/op 1.00
BenchmarkSync/memory_sync_1000_test 586687 ns/op 74312 B/op 2116 allocs/op 550044 ns/op 75882 B/op 2215 allocs/op 1.07
BenchmarkSync/memory_sync_1000_test - ns/op 586687 ns/op 550044 ns/op 1.07
BenchmarkSync/memory_sync_1000_test - B/op 74312 B/op 75882 B/op 0.98
BenchmarkSync/memory_sync_1000_test - allocs/op 2116 allocs/op 2215 allocs/op 0.96
BenchmarkSync/memory_sync_10000_test 7333122 ns/op 735112 B/op 20254 allocs/op 6980161 ns/op 734781 B/op 20264 allocs/op 1.05
BenchmarkSync/memory_sync_10000_test - ns/op 7333122 ns/op 6980161 ns/op 1.05
BenchmarkSync/memory_sync_10000_test - B/op 735112 B/op 734781 B/op 1.00
BenchmarkSync/memory_sync_10000_test - allocs/op 20254 allocs/op 20264 allocs/op 1.00
BenchmarkTextEditing 4791186583 ns/op 3982603312 B/op 20647719 allocs/op 5149924208 ns/op 3982589232 B/op 20647683 allocs/op 0.93
BenchmarkTextEditing - ns/op 4791186583 ns/op 5149924208 ns/op 0.93
BenchmarkTextEditing - B/op 3982603312 B/op 3982589232 B/op 1.00
BenchmarkTextEditing - allocs/op 20647719 allocs/op 20647683 allocs/op 1.00
BenchmarkTree/10000_vertices_to_protobuf 3597766 ns/op 6262982 B/op 70025 allocs/op 3506828 ns/op 6262994 B/op 70025 allocs/op 1.03
BenchmarkTree/10000_vertices_to_protobuf - ns/op 3597766 ns/op 3506828 ns/op 1.03
BenchmarkTree/10000_vertices_to_protobuf - B/op 6262982 B/op 6262994 B/op 1.00
BenchmarkTree/10000_vertices_to_protobuf - allocs/op 70025 allocs/op 70025 allocs/op 1
BenchmarkTree/10000_vertices_from_protobuf 155344460 ns/op 442171516 B/op 290039 allocs/op 151499584 ns/op 442174035 B/op 290053 allocs/op 1.03
BenchmarkTree/10000_vertices_from_protobuf - ns/op 155344460 ns/op 151499584 ns/op 1.03
BenchmarkTree/10000_vertices_from_protobuf - B/op 442171516 B/op 442174035 B/op 1.00
BenchmarkTree/10000_vertices_from_protobuf - allocs/op 290039 allocs/op 290053 allocs/op 1.00
BenchmarkTree/20000_vertices_to_protobuf 7530409 ns/op 12716976 B/op 140028 allocs/op 7726736 ns/op 12716868 B/op 140028 allocs/op 0.97
BenchmarkTree/20000_vertices_to_protobuf - ns/op 7530409 ns/op 7726736 ns/op 0.97
BenchmarkTree/20000_vertices_to_protobuf - B/op 12716976 B/op 12716868 B/op 1.00
BenchmarkTree/20000_vertices_to_protobuf - allocs/op 140028 allocs/op 140028 allocs/op 1
BenchmarkTree/20000_vertices_from_protobuf 691167379 ns/op 1697272728 B/op 580090 allocs/op 679242506 ns/op 1697265876 B/op 580063 allocs/op 1.02
BenchmarkTree/20000_vertices_from_protobuf - ns/op 691167379 ns/op 679242506 ns/op 1.02
BenchmarkTree/20000_vertices_from_protobuf - B/op 1697272728 B/op 1697265876 B/op 1.00
BenchmarkTree/20000_vertices_from_protobuf - allocs/op 580090 allocs/op 580063 allocs/op 1.00
BenchmarkTree/30000_vertices_to_protobuf 12423195 ns/op 19318246 B/op 210030 allocs/op 11807933 ns/op 19318330 B/op 210030 allocs/op 1.05
BenchmarkTree/30000_vertices_to_protobuf - ns/op 12423195 ns/op 11807933 ns/op 1.05
BenchmarkTree/30000_vertices_to_protobuf - B/op 19318246 B/op 19318330 B/op 1.00
BenchmarkTree/30000_vertices_to_protobuf - allocs/op 210030 allocs/op 210030 allocs/op 1
BenchmarkTree/30000_vertices_from_protobuf 1663614261 ns/op 3752052984 B/op 870053 allocs/op 1612661158 ns/op 3752036440 B/op 870050 allocs/op 1.03
BenchmarkTree/30000_vertices_from_protobuf - ns/op 1663614261 ns/op 1612661158 ns/op 1.03
BenchmarkTree/30000_vertices_from_protobuf - B/op 3752052984 B/op 3752036440 B/op 1.00
BenchmarkTree/30000_vertices_from_protobuf - allocs/op 870053 allocs/op 870050 allocs/op 1.00

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

coderabbitai[bot]
coderabbitai bot reviewed Oct 25, 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)
server/backend/database/database.go (1)

274-276: LGTM! Consider enhancing the documentation.

The method renaming improves clarity by removing the overly specific "AfterPushPull" suffix. This aligns well with the fix for the transaction deadlock issue by making it clear that this method has a more general purpose.

Consider expanding the method documentation to clarify:

  1. The transaction semantics (read vs write mode)
  2. The relationship with UpdateVersionVector
  3. The purpose of returning the minimum synced version vector

Example documentation:

-	// UpdateAndFindMinSyncedVersionVector updates the given serverSeq of the given client
-	// and returns the SyncedVersionVector of the document.
+	// UpdateAndFindMinSyncedVersionVector computes and returns the minimum synced version
+	// vector across all attached clients for the document. This operation requires only
+	// read access as the actual version vector update is handled by UpdateVersionVector.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1b88ea1 and aea5cd9.

📒 Files selected for processing (4)
  • server/backend/database/database.go (1 hunks)
  • server/backend/database/memory/database.go (2 hunks)
  • server/backend/database/mongo/client.go (1 hunks)
  • server/packs/packs.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/backend/database/memory/database.go
🔇 Additional comments (3)
server/packs/packs.go (1)

Line range hint 135-140: LGTM! Method name updated correctly.

The rename from UpdateAndFindMinSyncedVersionVectorAfterPushPull to UpdateAndFindMinSyncedVersionVector aligns with the PR's objective to fix transaction deadlock issues.

Let's verify the method name consistency across the codebase:

✅ Verification successful

Method name has been consistently updated across the codebase

The verification confirms that:

  • The old method name UpdateAndFindMinSyncedVersionVectorAfterPushPull has been completely removed from the codebase
  • The new method name UpdateAndFindMinSyncedVersionVector is consistently used across all relevant files:
    • Interface definition in database.go
    • Implementation in memory/database.go
    • Implementation in mongo/client.go
    • Usage in packs/packs.go
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the old method name is completely replaced
# Test: Search for any remaining instances of the old name
rg "UpdateAndFindMinSyncedVersionVectorAfterPushPull"

# Test: Verify the new name is used consistently
rg "UpdateAndFindMinSyncedVersionVector"

Length of output: 819

server/backend/database/mongo/client.go (2)

1200-1201: Method rename improves clarity.

The rename from UpdateAndFindMinSyncedVersionVectorAfterPushPull to UpdateAndFindMinSyncedVersionVector makes the method name more concise while maintaining clarity about its purpose.

Line range hint 1201-1275: Implementation looks good and addresses the deadlock issue.

The implementation properly handles the computation of minimum version vector while addressing the transaction deadlock issue mentioned in the PR objectives. Key improvements:

  • No longer starts in write mode
  • Properly filters out detached clients
  • Clear separation between reading version vectors and updating the current client's vector

Let's verify the usage of this method:

✅ Verification successful

Implementation is correctly used and consistent across the codebase

The verification confirms:

  • The memory database implementation follows the same pattern as mongo implementation, using read-only transaction (txn := d.db.Txn(false))
  • The method is used correctly in packs.go for garbage collection after push-pull operations
  • No references to the old method name exist in the codebase
  • The implementation consistently handles version vector updates and filtering of detached clients
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the method is called correctly and not in write mode
# Test: Search for calls to this method to ensure they're updated
rg -A 3 "UpdateAndFindMinSyncedVersionVector"

# Test: Search for any remaining references to old method name
rg "UpdateAndFindMinSyncedVersionVectorAfterPushPull"

Length of output: 1728

Script:

#!/bin/bash
# Let's check the implementation in memory database to ensure consistency
cat server/backend/database/memory/database.go | grep -A 50 "UpdateAndFindMinSyncedVersionVector"

# Let's also check how it's used in packs.go for context
cat server/packs/packs.go | grep -B 10 -A 10 "UpdateAndFindMinSyncedVersionVector"

Length of output: 2459

@hackerwins hackerwins merged commit fac4779 into main Oct 25, 2024
5 checks passed
@hackerwins hackerwins deleted the fix-invalid-tx-usage branch October 25, 2024 10:04
@BrewTestBot BrewTestBot mentioned this pull request Oct 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

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

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

Successfully merging this pull request may close these issues.
1 participant
@hackerwins