Clear presence when housekeeping process #936

raararaara · 2024-07-22T02:23:45Z

What this PR does / why we need it?

This PR addresses the absence of presence clearing logic in document detach during client deactivation for Housekeeping compared to document detach by the SDK. By implementing server-side presence clearance, this PR ensures that presence is correctly handled during the housekeeping process.

Any background context you want to provide?

The changes include:

Performing document detach with client deactivation and updating the SyncedSeq solely through the database -> making an API call similar to the SDK's document detach
This change allows presence to be cleared through client.detach() during deactivation and detach processes, simplifying the process and omitting the need for updating SyncedSeq separately.
In case of deactivation failure, some documents may not detach completely, but another deactivation attempt is possible.
Consideration for clustering and sharding per document
Taking into account document structures sharded across clusters by keys, the update allows one server to act as a client while multiple servers can perform detach operations.

What are the relevant tickets?

Fixes #602

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
- Introduced methods for simulating document attachment and client activation, enhancing server-side operations.
- Added functionality for converting internal documents to public documents, improving document management.
- Enhanced client connection capabilities, allowing for improved document and client interaction.
- Implemented configuration options for dynamic gateway service referencing in Kubernetes.
- Added command-line flags for backend gateway address configuration, increasing server flexibility.
Bug Fixes
- Improved error handling for detach requests to ensure system integrity in case of failures.
- Enhanced logic for client deactivation, ensuring the most up-to-date client state is utilized.
Chores
- Streamlined methods for client deactivation, simplifying document status management during operations.
- Expanded configuration options in the Helm chart for better service adaptability.
- Added new constants and configuration fields to improve server setup and management.

coderabbitai · 2024-07-22T02:23:52Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This update enhances error handling and control flow during client detachment and deactivation processes, ensuring system consistency and integrity. Key enhancements include new methods for simulating document attachment and activation, improved backend interactions during client deactivation, and effective presence management. The housekeeping process has been refined to clean up presence more efficiently, addressing potential growth in the presence snapshot.

Changes

File	Change Summary
client/client.go	Added `PretendAttach` and `PretendActivate` methods for simulating attachment and activation; improved error handling in the `Detach` method.
pkg/document/document.go	Introduced `ToDocument` function for converting `InternalDocument` to `Document`; added `setInternalDoc` for better internal document management.
pkg/document/internal_document.go	Added `SyncCheckpoint` and `ToDocument` methods to enhance synchronization and conversion functionalities.
server/clients/clients.go	Modified `Deactivate` function to utilize `*backend.Backend`, ensuring proper client info retrieval and document update logic.
server/rpc/yorkie_server.go	Updated `DeactivateClient` to use `s.backend` instead of `s.backend.DB`, improving backend interactions.
server/server.go	Altered `DeactivateClient` and `RegisterHousekeepingTasks` methods to integrate `rpcAddr`, enhancing client management and housekeeping task registration.
build/charts/yorkie-cluster/templates/yorkie/deployment.yaml	Added `GATEWAY_HOST` environment variable to Kubernetes deployment for flexible service discovery.
build/charts/yorkie-cluster/values.yaml	Introduced a new `gateway` configuration option to enhance service routing capabilities.

Assessment against linked issues

Objective	Addressed	Explanation
Handle Presence Cleanup in Housekeeping Process (#602)	✅
Ensure presence is cleared during document detachment	✅	Presence is now correctly managed during detachment processes.

🐇 In warren deep, where shadows play,
Changes weave a bright array.
With presence clear, and logic sound,
The rabbit hops where joy is found!
🥕✨

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.

github-actions

Go Benchmark

Benchmark suite	Current: `62a461e`	Previous: `4ae640a`	Ratio
`BenchmarkDocument/constructor_test`	`1493` ns/op 1337 B/op 24 allocs/op	`1549` ns/op 1337 B/op 24 allocs/op	`0.96`
`BenchmarkDocument/constructor_test - ns/op`	`1493` ns/op	`1549` ns/op	`0.96`
`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`	`966.6` ns/op 1305 B/op 22 allocs/op	`969.3` ns/op 1305 B/op 22 allocs/op	`1.00`
`BenchmarkDocument/status_test - ns/op`	`966.6` ns/op	`969.3` ns/op	`1.00`
`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`	`7676` ns/op 7273 B/op 132 allocs/op	`7797` ns/op 7273 B/op 132 allocs/op	`0.98`
`BenchmarkDocument/equals_test - ns/op`	`7676` ns/op	`7797` ns/op	`0.98`
`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`	`16774` ns/op 12139 B/op 262 allocs/op	`20586` ns/op 12138 B/op 262 allocs/op	`0.81`
`BenchmarkDocument/nested_update_test - ns/op`	`16774` ns/op	`20586` ns/op	`0.81`
`BenchmarkDocument/nested_update_test - B/op`	`12139` B/op	`12138` B/op	`1.00`
`BenchmarkDocument/nested_update_test - allocs/op`	`262` allocs/op	`262` allocs/op	`1`
`BenchmarkDocument/delete_test`	`22483` ns/op 15364 B/op 341 allocs/op	`23144` ns/op 15363 B/op 341 allocs/op	`0.97`
`BenchmarkDocument/delete_test - ns/op`	`22483` ns/op	`23144` ns/op	`0.97`
`BenchmarkDocument/delete_test - B/op`	`15364` B/op	`15363` B/op	`1.00`
`BenchmarkDocument/delete_test - allocs/op`	`341` allocs/op	`341` allocs/op	`1`
`BenchmarkDocument/object_test`	`8564` ns/op 6817 B/op 120 allocs/op	`8717` ns/op 6817 B/op 120 allocs/op	`0.98`
`BenchmarkDocument/object_test - ns/op`	`8564` ns/op	`8717` ns/op	`0.98`
`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`	`32031` ns/op 11947 B/op 276 allocs/op	`29360` ns/op 11947 B/op 276 allocs/op	`1.09`
`BenchmarkDocument/array_test - ns/op`	`32031` ns/op	`29360` ns/op	`1.09`
`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`	`32015` ns/op 14779 B/op 486 allocs/op	`30837` ns/op 14715 B/op 469 allocs/op	`1.04`
`BenchmarkDocument/text_test - ns/op`	`32015` ns/op	`30837` ns/op	`1.04`
`BenchmarkDocument/text_test - B/op`	`14779` B/op	`14715` B/op	`1.00`
`BenchmarkDocument/text_test - allocs/op`	`486` allocs/op	`469` allocs/op	`1.04`
`BenchmarkDocument/text_composition_test`	`30032` ns/op 18462 B/op 502 allocs/op	`29504` ns/op 18423 B/op 484 allocs/op	`1.02`
`BenchmarkDocument/text_composition_test - ns/op`	`30032` ns/op	`29504` ns/op	`1.02`
`BenchmarkDocument/text_composition_test - B/op`	`18462` B/op	`18423` B/op	`1.00`
`BenchmarkDocument/text_composition_test - allocs/op`	`502` allocs/op	`484` allocs/op	`1.04`
`BenchmarkDocument/rich_text_test`	`82249` ns/op 38708 B/op 1165 allocs/op	`82340` ns/op 38477 B/op 1148 allocs/op	`1.00`
`BenchmarkDocument/rich_text_test - ns/op`	`82249` ns/op	`82340` ns/op	`1.00`
`BenchmarkDocument/rich_text_test - B/op`	`38708` B/op	`38477` B/op	`1.01`
`BenchmarkDocument/rich_text_test - allocs/op`	`1165` allocs/op	`1148` allocs/op	`1.01`
`BenchmarkDocument/counter_test`	`17628` ns/op 10722 B/op 244 allocs/op	`17783` ns/op 10722 B/op 244 allocs/op	`0.99`
`BenchmarkDocument/counter_test - ns/op`	`17628` ns/op	`17783` ns/op	`0.99`
`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`	`1322036` ns/op 872054 B/op 17276 allocs/op	`1297144` ns/op 870921 B/op 16753 allocs/op	`1.02`
`BenchmarkDocument/text_edit_gc_100 - ns/op`	`1322036` ns/op	`1297144` ns/op	`1.02`
`BenchmarkDocument/text_edit_gc_100 - B/op`	`872054` B/op	`870921` B/op	`1.00`
`BenchmarkDocument/text_edit_gc_100 - allocs/op`	`17276` allocs/op	`16753` allocs/op	`1.03`
`BenchmarkDocument/text_edit_gc_1000`	`50700415` ns/op 50546456 B/op 186728 allocs/op	`50024644` ns/op 50535262 B/op 181716 allocs/op	`1.01`
`BenchmarkDocument/text_edit_gc_1000 - ns/op`	`50700415` ns/op	`50024644` ns/op	`1.01`
`BenchmarkDocument/text_edit_gc_1000 - B/op`	`50546456` B/op	`50535262` B/op	`1.00`
`BenchmarkDocument/text_edit_gc_1000 - allocs/op`	`186728` allocs/op	`181716` allocs/op	`1.03`
`BenchmarkDocument/text_split_gc_100`	`1917138` ns/op 1588547 B/op 15945 allocs/op	`1901148` ns/op 1528771 B/op 15604 allocs/op	`1.01`
`BenchmarkDocument/text_split_gc_100 - ns/op`	`1917138` ns/op	`1901148` ns/op	`1.01`
`BenchmarkDocument/text_split_gc_100 - B/op`	`1588547` B/op	`1528771` B/op	`1.04`
`BenchmarkDocument/text_split_gc_100 - allocs/op`	`15945` allocs/op	`15604` allocs/op	`1.02`
`BenchmarkDocument/text_split_gc_1000`	`115560474` ns/op 141482358 B/op 186145 allocs/op	`113539833` ns/op 135077032 B/op 182196 allocs/op	`1.02`
`BenchmarkDocument/text_split_gc_1000 - ns/op`	`115560474` ns/op	`113539833` ns/op	`1.02`
`BenchmarkDocument/text_split_gc_1000 - B/op`	`141482358` B/op	`135077032` B/op	`1.05`
`BenchmarkDocument/text_split_gc_1000 - allocs/op`	`186145` allocs/op	`182196` allocs/op	`1.02`
`BenchmarkDocument/text_delete_all_10000`	`16739050` ns/op 10214147 B/op 55683 allocs/op	`16926453` ns/op 10183237 B/op 40674 allocs/op	`0.99`
`BenchmarkDocument/text_delete_all_10000 - ns/op`	`16739050` ns/op	`16926453` ns/op	`0.99`
`BenchmarkDocument/text_delete_all_10000 - B/op`	`10214147` B/op	`10183237` B/op	`1.00`
`BenchmarkDocument/text_delete_all_10000 - allocs/op`	`55683` allocs/op	`40674` allocs/op	`1.37`
`BenchmarkDocument/text_delete_all_100000`	`267238895` ns/op 142986336 B/op 561725 allocs/op	`308578662` ns/op 142678592 B/op 411701 allocs/op	`0.87`
`BenchmarkDocument/text_delete_all_100000 - ns/op`	`267238895` ns/op	`308578662` ns/op	`0.87`
`BenchmarkDocument/text_delete_all_100000 - B/op`	`142986336` B/op	`142678592` B/op	`1.00`
`BenchmarkDocument/text_delete_all_100000 - allocs/op`	`561725` allocs/op	`411701` allocs/op	`1.36`
`BenchmarkDocument/text_100`	`228594` ns/op 120235 B/op 5180 allocs/op	`216580` ns/op 120037 B/op 5081 allocs/op	`1.06`
`BenchmarkDocument/text_100 - ns/op`	`228594` ns/op	`216580` ns/op	`1.06`
`BenchmarkDocument/text_100 - B/op`	`120235` B/op	`120037` B/op	`1.00`
`BenchmarkDocument/text_100 - allocs/op`	`5180` allocs/op	`5081` allocs/op	`1.02`
`BenchmarkDocument/text_1000`	`2492983` ns/op 1171026 B/op 51084 allocs/op	`2365268` ns/op 1169023 B/op 50085 allocs/op	`1.05`
`BenchmarkDocument/text_1000 - ns/op`	`2492983` ns/op	`2365268` ns/op	`1.05`
`BenchmarkDocument/text_1000 - B/op`	`1171026` B/op	`1169023` B/op	`1.00`
`BenchmarkDocument/text_1000 - allocs/op`	`51084` allocs/op	`50085` allocs/op	`1.02`
`BenchmarkDocument/array_1000`	`1281845` ns/op 1091367 B/op 11831 allocs/op	`1233259` ns/op 1091316 B/op 11831 allocs/op	`1.04`
`BenchmarkDocument/array_1000 - ns/op`	`1281845` ns/op	`1233259` ns/op	`1.04`
`BenchmarkDocument/array_1000 - B/op`	`1091367` B/op	`1091316` B/op	`1.00`
`BenchmarkDocument/array_1000 - allocs/op`	`11831` allocs/op	`11831` allocs/op	`1`
`BenchmarkDocument/array_10000`	`13249848` ns/op 9800097 B/op 120296 allocs/op	`13201619` ns/op 9799969 B/op 120296 allocs/op	`1.00`
`BenchmarkDocument/array_10000 - ns/op`	`13249848` ns/op	`13201619` ns/op	`1.00`
`BenchmarkDocument/array_10000 - B/op`	`9800097` B/op	`9799969` B/op	`1.00`
`BenchmarkDocument/array_10000 - allocs/op`	`120296` allocs/op	`120296` allocs/op	`1`
`BenchmarkDocument/array_gc_100`	`153256` ns/op 132732 B/op 1261 allocs/op	`148310` ns/op 132724 B/op 1261 allocs/op	`1.03`
`BenchmarkDocument/array_gc_100 - ns/op`	`153256` ns/op	`148310` ns/op	`1.03`
`BenchmarkDocument/array_gc_100 - B/op`	`132732` B/op	`132724` B/op	`1.00`
`BenchmarkDocument/array_gc_100 - allocs/op`	`1261` allocs/op	`1261` allocs/op	`1`
`BenchmarkDocument/array_gc_1000`	`1450776` ns/op 1159201 B/op 12877 allocs/op	`1396440` ns/op 1159102 B/op 12876 allocs/op	`1.04`
`BenchmarkDocument/array_gc_1000 - ns/op`	`1450776` ns/op	`1396440` ns/op	`1.04`
`BenchmarkDocument/array_gc_1000 - B/op`	`1159201` B/op	`1159102` B/op	`1.00`
`BenchmarkDocument/array_gc_1000 - allocs/op`	`12877` allocs/op	`12876` allocs/op	`1.00`
`BenchmarkDocument/counter_1000`	`213963` ns/op 193081 B/op 5771 allocs/op	`202334` ns/op 193080 B/op 5771 allocs/op	`1.06`
`BenchmarkDocument/counter_1000 - ns/op`	`213963` ns/op	`202334` ns/op	`1.06`
`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`	`2224434` ns/op 2088000 B/op 59778 allocs/op	`2199700` ns/op 2088013 B/op 59778 allocs/op	`1.01`
`BenchmarkDocument/counter_10000 - ns/op`	`2224434` ns/op	`2199700` ns/op	`1.01`
`BenchmarkDocument/counter_10000 - B/op`	`2088000` B/op	`2088013` B/op	`1.00`
`BenchmarkDocument/counter_10000 - allocs/op`	`59778` allocs/op	`59778` allocs/op	`1`
`BenchmarkDocument/object_1000`	`1444469` ns/op 1428181 B/op 9849 allocs/op	`1413682` ns/op 1428153 B/op 9849 allocs/op	`1.02`
`BenchmarkDocument/object_1000 - ns/op`	`1444469` ns/op	`1413682` ns/op	`1.02`
`BenchmarkDocument/object_1000 - B/op`	`1428181` B/op	`1428153` B/op	`1.00`
`BenchmarkDocument/object_1000 - allocs/op`	`9849` allocs/op	`9849` allocs/op	`1`
`BenchmarkDocument/object_10000`	`15143318` ns/op 12165522 B/op 100560 allocs/op	`15783586` ns/op 12166732 B/op 100564 allocs/op	`0.96`
`BenchmarkDocument/object_10000 - ns/op`	`15143318` ns/op	`15783586` ns/op	`0.96`
`BenchmarkDocument/object_10000 - B/op`	`12165522` B/op	`12166732` B/op	`1.00`
`BenchmarkDocument/object_10000 - allocs/op`	`100560` allocs/op	`100564` allocs/op	`1.00`
`BenchmarkDocument/tree_100`	`1065146` ns/op 943704 B/op 6101 allocs/op	`1047638` ns/op 943702 B/op 6101 allocs/op	`1.02`
`BenchmarkDocument/tree_100 - ns/op`	`1065146` ns/op	`1047638` ns/op	`1.02`
`BenchmarkDocument/tree_100 - B/op`	`943704` B/op	`943702` B/op	`1.00`
`BenchmarkDocument/tree_100 - allocs/op`	`6101` allocs/op	`6101` allocs/op	`1`
`BenchmarkDocument/tree_1000`	`78507841` ns/op 86460412 B/op 60114 allocs/op	`77148387` ns/op 86460333 B/op 60115 allocs/op	`1.02`
`BenchmarkDocument/tree_1000 - ns/op`	`78507841` ns/op	`77148387` ns/op	`1.02`
`BenchmarkDocument/tree_1000 - B/op`	`86460412` B/op	`86460333` B/op	`1.00`
`BenchmarkDocument/tree_1000 - allocs/op`	`60114` allocs/op	`60115` allocs/op	`1.00`
`BenchmarkDocument/tree_10000`	`9577765926` ns/op 8580651184 B/op 600212 allocs/op	`9715671041` ns/op 8580668976 B/op 600226 allocs/op	`0.99`
`BenchmarkDocument/tree_10000 - ns/op`	`9577765926` ns/op	`9715671041` ns/op	`0.99`
`BenchmarkDocument/tree_10000 - B/op`	`8580651184` B/op	`8580668976` B/op	`1.00`
`BenchmarkDocument/tree_10000 - allocs/op`	`600212` allocs/op	`600226` allocs/op	`1.00`
`BenchmarkDocument/tree_delete_all_1000`	`80493557` ns/op 87509590 B/op 75264 allocs/op	`80678138` ns/op 87509409 B/op 75264 allocs/op	`1.00`
`BenchmarkDocument/tree_delete_all_1000 - ns/op`	`80493557` ns/op	`80678138` ns/op	`1.00`
`BenchmarkDocument/tree_delete_all_1000 - B/op`	`87509590` B/op	`87509409` B/op	`1.00`
`BenchmarkDocument/tree_delete_all_1000 - allocs/op`	`75264` allocs/op	`75264` allocs/op	`1`
`BenchmarkDocument/tree_edit_gc_100`	`4043989` ns/op 4146711 B/op 15141 allocs/op	`3960878` ns/op 4146779 B/op 15141 allocs/op	`1.02`
`BenchmarkDocument/tree_edit_gc_100 - ns/op`	`4043989` ns/op	`3960878` ns/op	`1.02`
`BenchmarkDocument/tree_edit_gc_100 - B/op`	`4146711` B/op	`4146779` B/op	`1.00`
`BenchmarkDocument/tree_edit_gc_100 - allocs/op`	`15141` allocs/op	`15141` allocs/op	`1`
`BenchmarkDocument/tree_edit_gc_1000`	`320924601` ns/op 383746236 B/op 154868 allocs/op	`314875560` ns/op 383824892 B/op 154860 allocs/op	`1.02`
`BenchmarkDocument/tree_edit_gc_1000 - ns/op`	`320924601` ns/op	`314875560` ns/op	`1.02`
`BenchmarkDocument/tree_edit_gc_1000 - B/op`	`383746236` B/op	`383824892` B/op	`1.00`
`BenchmarkDocument/tree_edit_gc_1000 - allocs/op`	`154868` allocs/op	`154860` allocs/op	`1.00`
`BenchmarkDocument/tree_split_gc_100`	`2659686` ns/op 2412451 B/op 11125 allocs/op	`2600369` ns/op 2412515 B/op 11125 allocs/op	`1.02`
`BenchmarkDocument/tree_split_gc_100 - ns/op`	`2659686` ns/op	`2600369` ns/op	`1.02`
`BenchmarkDocument/tree_split_gc_100 - B/op`	`2412451` B/op	`2412515` B/op	`1.00`
`BenchmarkDocument/tree_split_gc_100 - allocs/op`	`11125` allocs/op	`11125` allocs/op	`1`
`BenchmarkDocument/tree_split_gc_1000`	`191790176` ns/op 222252912 B/op 122002 allocs/op	`189117419` ns/op 222249912 B/op 121990 allocs/op	`1.01`
`BenchmarkDocument/tree_split_gc_1000 - ns/op`	`191790176` ns/op	`189117419` ns/op	`1.01`
`BenchmarkDocument/tree_split_gc_1000 - B/op`	`222252912` B/op	`222249912` B/op	`1.00`
`BenchmarkDocument/tree_split_gc_1000 - allocs/op`	`122002` allocs/op	`121990` allocs/op	`1.00`
`BenchmarkRPC/attach_large_document`	`2357187177` ns/op 3534213184 B/op 13223 allocs/op	`2231916118` ns/op 3604715200 B/op 14410 allocs/op	`1.06`
`BenchmarkRPC/attach_large_document - ns/op`	`2357187177` ns/op	`2231916118` ns/op	`1.06`
`BenchmarkRPC/attach_large_document - B/op`	`3534213184` B/op	`3604715200` B/op	`0.98`
`BenchmarkRPC/attach_large_document - allocs/op`	`13223` allocs/op	`14410` allocs/op	`0.92`
`BenchmarkRPC/adminCli_to_server`	`513814480` ns/op 35958052 B/op 289568 allocs/op	`539083220` ns/op 35949996 B/op 289538 allocs/op	`0.95`
`BenchmarkRPC/adminCli_to_server - ns/op`	`513814480` ns/op	`539083220` ns/op	`0.95`
`BenchmarkRPC/adminCli_to_server - B/op`	`35958052` B/op	`35949996` B/op	`1.00`
`BenchmarkRPC/adminCli_to_server - allocs/op`	`289568` allocs/op	`289538` allocs/op	`1.00`
`BenchmarkLocker`	`63.97` ns/op 16 B/op 1 allocs/op	`64.48` ns/op 16 B/op 1 allocs/op	`0.99`
`BenchmarkLocker - ns/op`	`63.97` ns/op	`64.48` 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`	`40.15` ns/op 0 B/op 0 allocs/op	`39.78` ns/op 0 B/op 0 allocs/op	`1.01`
`BenchmarkLockerParallel - ns/op`	`40.15` ns/op	`39.78` 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`	`150.7` ns/op 15 B/op 0 allocs/op	`162.3` ns/op 15 B/op 0 allocs/op	`0.93`
`BenchmarkLockerMoreKeys - ns/op`	`150.7` ns/op	`162.3` ns/op	`0.93`
`BenchmarkLockerMoreKeys - B/op`	`15` B/op	`15` B/op	`1`
`BenchmarkLockerMoreKeys - allocs/op`	`0` allocs/op	`0` allocs/op	`1`
`BenchmarkChange/Push_10_Changes`	`3682626` ns/op 118173 B/op 1215 allocs/op	`3875350` ns/op 116948 B/op 1214 allocs/op	`0.95`
`BenchmarkChange/Push_10_Changes - ns/op`	`3682626` ns/op	`3875350` ns/op	`0.95`
`BenchmarkChange/Push_10_Changes - B/op`	`118173` B/op	`116948` B/op	`1.01`
`BenchmarkChange/Push_10_Changes - allocs/op`	`1215` allocs/op	`1214` allocs/op	`1.00`
`BenchmarkChange/Push_100_Changes`	`14949082` ns/op 568861 B/op 6585 allocs/op	`15938762` ns/op 571609 B/op 6585 allocs/op	`0.94`
`BenchmarkChange/Push_100_Changes - ns/op`	`14949082` ns/op	`15938762` ns/op	`0.94`
`BenchmarkChange/Push_100_Changes - B/op`	`568861` B/op	`571609` B/op	`1.00`
`BenchmarkChange/Push_100_Changes - allocs/op`	`6585` allocs/op	`6585` allocs/op	`1`
`BenchmarkChange/Push_1000_Changes`	`123324018` ns/op 5363270 B/op 63079 allocs/op	`128283783` ns/op 5390909 B/op 63081 allocs/op	`0.96`
`BenchmarkChange/Push_1000_Changes - ns/op`	`123324018` ns/op	`128283783` ns/op	`0.96`
`BenchmarkChange/Push_1000_Changes - B/op`	`5363270` B/op	`5390909` B/op	`0.99`
`BenchmarkChange/Push_1000_Changes - allocs/op`	`63079` allocs/op	`63081` allocs/op	`1.00`
`BenchmarkChange/Pull_10_Changes`	`2943348` ns/op 103558 B/op 1019 allocs/op	`3085917` ns/op 102318 B/op 1019 allocs/op	`0.95`
`BenchmarkChange/Pull_10_Changes - ns/op`	`2943348` ns/op	`3085917` ns/op	`0.95`
`BenchmarkChange/Pull_10_Changes - B/op`	`103558` B/op	`102318` B/op	`1.01`
`BenchmarkChange/Pull_10_Changes - allocs/op`	`1019` allocs/op	`1019` allocs/op	`1`
`BenchmarkChange/Pull_100_Changes`	`4481123` ns/op 269354 B/op 3489 allocs/op	`4653244` ns/op 266393 B/op 3489 allocs/op	`0.96`
`BenchmarkChange/Pull_100_Changes - ns/op`	`4481123` ns/op	`4653244` ns/op	`0.96`
`BenchmarkChange/Pull_100_Changes - B/op`	`269354` B/op	`266393` B/op	`1.01`
`BenchmarkChange/Pull_100_Changes - allocs/op`	`3489` allocs/op	`3489` allocs/op	`1`
`BenchmarkChange/Pull_1000_Changes`	`8506961` ns/op 1497032 B/op 29874 allocs/op	`8734569` ns/op 1489803 B/op 29867 allocs/op	`0.97`
`BenchmarkChange/Pull_1000_Changes - ns/op`	`8506961` ns/op	`8734569` ns/op	`0.97`
`BenchmarkChange/Pull_1000_Changes - B/op`	`1497032` B/op	`1489803` B/op	`1.00`
`BenchmarkChange/Pull_1000_Changes - allocs/op`	`29874` allocs/op	`29867` allocs/op	`1.00`
`BenchmarkSnapshot/Push_3KB_snapshot`	`17585267` ns/op 713327 B/op 6587 allocs/op	`18691314` ns/op 710356 B/op 6587 allocs/op	`0.94`
`BenchmarkSnapshot/Push_3KB_snapshot - ns/op`	`17585267` ns/op	`18691314` ns/op	`0.94`
`BenchmarkSnapshot/Push_3KB_snapshot - B/op`	`713327` B/op	`710356` B/op	`1.00`
`BenchmarkSnapshot/Push_3KB_snapshot - allocs/op`	`6587` allocs/op	`6587` allocs/op	`1`
`BenchmarkSnapshot/Push_30KB_snapshot`	`125111715` ns/op 5638135 B/op 63079 allocs/op	`129614800` ns/op 5588012 B/op 63080 allocs/op	`0.97`
`BenchmarkSnapshot/Push_30KB_snapshot - ns/op`	`125111715` ns/op	`129614800` ns/op	`0.97`
`BenchmarkSnapshot/Push_30KB_snapshot - B/op`	`5638135` B/op	`5588012` B/op	`1.01`
`BenchmarkSnapshot/Push_30KB_snapshot - allocs/op`	`63079` allocs/op	`63080` allocs/op	`1.00`
`BenchmarkSnapshot/Pull_3KB_snapshot`	`6640854` ns/op 925428 B/op 15523 allocs/op	`6800608` ns/op 921145 B/op 15522 allocs/op	`0.98`
`BenchmarkSnapshot/Pull_3KB_snapshot - ns/op`	`6640854` ns/op	`6800608` ns/op	`0.98`
`BenchmarkSnapshot/Pull_3KB_snapshot - B/op`	`925428` B/op	`921145` B/op	`1.00`
`BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op`	`15523` allocs/op	`15522` allocs/op	`1.00`
`BenchmarkSnapshot/Pull_30KB_snapshot`	`15550003` ns/op 7161403 B/op 150121 allocs/op	`15443953` ns/op 7150322 B/op 150120 allocs/op	`1.01`
`BenchmarkSnapshot/Pull_30KB_snapshot - ns/op`	`15550003` ns/op	`15443953` ns/op	`1.01`
`BenchmarkSnapshot/Pull_30KB_snapshot - B/op`	`7161403` B/op	`7150322` B/op	`1.00`
`BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op`	`150121` allocs/op	`150120` allocs/op	`1.00`
`BenchmarkSplayTree/stress_test_100000`	`0.19` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/stress_test_100000 - ns/op`	`0.19` ns/op
`BenchmarkSplayTree/stress_test_100000 - B/op`	`0` B/op
`BenchmarkSplayTree/stress_test_100000 - allocs/op`	`0` allocs/op
`BenchmarkSplayTree/stress_test_200000`	`0.3836` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/stress_test_200000 - ns/op`	`0.3836` ns/op
`BenchmarkSplayTree/stress_test_200000 - B/op`	`0` B/op
`BenchmarkSplayTree/stress_test_200000 - allocs/op`	`0` allocs/op
`BenchmarkSplayTree/stress_test_300000`	`0.6054` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/stress_test_300000 - ns/op`	`0.6054` ns/op
`BenchmarkSplayTree/stress_test_300000 - B/op`	`0` B/op
`BenchmarkSplayTree/stress_test_300000 - allocs/op`	`0` allocs/op
`BenchmarkSplayTree/random_access_100000`	`0.01253` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/random_access_100000 - ns/op`	`0.01253` ns/op
`BenchmarkSplayTree/random_access_100000 - B/op`	`0` B/op
`BenchmarkSplayTree/random_access_100000 - allocs/op`	`0` allocs/op
`BenchmarkSplayTree/random_access_200000`	`0.02357` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/random_access_200000 - ns/op`	`0.02357` ns/op
`BenchmarkSplayTree/random_access_200000 - B/op`	`0` B/op
`BenchmarkSplayTree/random_access_200000 - allocs/op`	`0` allocs/op
`BenchmarkSplayTree/random_access_300000`	`0.0361` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/random_access_300000 - ns/op`	`0.0361` ns/op
`BenchmarkSplayTree/random_access_300000 - B/op`	`0` B/op
`BenchmarkSplayTree/random_access_300000 - allocs/op`	`0` allocs/op
`BenchmarkSplayTree/editing_trace_bench`	`0.001779` ns/op 0 B/op 0 allocs/op
`BenchmarkSplayTree/editing_trace_bench - ns/op`	`0.001779` ns/op
`BenchmarkSplayTree/editing_trace_bench - B/op`	`0` B/op
`BenchmarkSplayTree/editing_trace_bench - allocs/op`	`0` allocs/op
`BenchmarkSync/memory_sync_10_test`	`6767` ns/op 1286 B/op 38 allocs/op	`7266` ns/op 1286 B/op 38 allocs/op	`0.93`
`BenchmarkSync/memory_sync_10_test - ns/op`	`6767` ns/op	`7266` ns/op	`0.93`
`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`	`51424` ns/op 8636 B/op 273 allocs/op	`51934` ns/op 8632 B/op 272 allocs/op	`0.99`
`BenchmarkSync/memory_sync_100_test - ns/op`	`51424` ns/op	`51934` ns/op	`0.99`
`BenchmarkSync/memory_sync_100_test - B/op`	`8636` B/op	`8632` B/op	`1.00`
`BenchmarkSync/memory_sync_100_test - allocs/op`	`273` allocs/op	`272` allocs/op	`1.00`
`BenchmarkSync/memory_sync_1000_test`	`585019` ns/op 74061 B/op 2103 allocs/op	`587913` ns/op 74229 B/op 2113 allocs/op	`1.00`
`BenchmarkSync/memory_sync_1000_test - ns/op`	`585019` ns/op	`587913` ns/op	`1.00`
`BenchmarkSync/memory_sync_1000_test - B/op`	`74061` B/op	`74229` B/op	`1.00`
`BenchmarkSync/memory_sync_1000_test - allocs/op`	`2103` allocs/op	`2113` allocs/op	`1.00`
`BenchmarkSync/memory_sync_10000_test`	`6988058` ns/op 737708 B/op 20286 allocs/op	`7400660` ns/op 735273 B/op 20214 allocs/op	`0.94`
`BenchmarkSync/memory_sync_10000_test - ns/op`	`6988058` ns/op	`7400660` ns/op	`0.94`
`BenchmarkSync/memory_sync_10000_test - B/op`	`737708` B/op	`735273` B/op	`1.00`
`BenchmarkSync/memory_sync_10000_test - allocs/op`	`20286` allocs/op	`20214` allocs/op	`1.00`
`BenchmarkTextEditing`	`4916989351` ns/op 3903596192 B/op 19608433 allocs/op	`4889188948` ns/op 3901924192 B/op 18743195 allocs/op	`1.01`
`BenchmarkTextEditing - ns/op`	`4916989351` ns/op	`4889188948` ns/op	`1.01`
`BenchmarkTextEditing - B/op`	`3903596192` B/op	`3901924192` B/op	`1.00`
`BenchmarkTextEditing - allocs/op`	`19608433` allocs/op	`18743195` allocs/op	`1.05`
`BenchmarkTree/10000_vertices_to_protobuf`	`3420002` ns/op 6262973 B/op 70025 allocs/op	`3446065` ns/op 6263016 B/op 70025 allocs/op	`0.99`
`BenchmarkTree/10000_vertices_to_protobuf - ns/op`	`3420002` ns/op	`3446065` ns/op	`0.99`
`BenchmarkTree/10000_vertices_to_protobuf - B/op`	`6262973` B/op	`6263016` B/op	`1.00`
`BenchmarkTree/10000_vertices_to_protobuf - allocs/op`	`70025` allocs/op	`70025` allocs/op	`1`
`BenchmarkTree/10000_vertices_from_protobuf`	`150716951` ns/op 442174296 B/op 290067 allocs/op	`149737156` ns/op 442171406 B/op 290038 allocs/op	`1.01`
`BenchmarkTree/10000_vertices_from_protobuf - ns/op`	`150716951` ns/op	`149737156` ns/op	`1.01`
`BenchmarkTree/10000_vertices_from_protobuf - B/op`	`442174296` B/op	`442171406` B/op	`1.00`
`BenchmarkTree/10000_vertices_from_protobuf - allocs/op`	`290067` allocs/op	`290038` allocs/op	`1.00`
`BenchmarkTree/20000_vertices_to_protobuf`	`7629835` ns/op 12716976 B/op 140028 allocs/op	`7485911` ns/op 12716915 B/op 140028 allocs/op	`1.02`
`BenchmarkTree/20000_vertices_to_protobuf - ns/op`	`7629835` ns/op	`7485911` ns/op	`1.02`
`BenchmarkTree/20000_vertices_to_protobuf - B/op`	`12716976` B/op	`12716915` B/op	`1.00`
`BenchmarkTree/20000_vertices_to_protobuf - allocs/op`	`140028` allocs/op	`140028` allocs/op	`1`
`BenchmarkTree/20000_vertices_from_protobuf`	`677640303` ns/op 1697264112 B/op 580046 allocs/op	`677321812` ns/op 1697267944 B/op 580043 allocs/op	`1.00`
`BenchmarkTree/20000_vertices_from_protobuf - ns/op`	`677640303` ns/op	`677321812` ns/op	`1.00`
`BenchmarkTree/20000_vertices_from_protobuf - B/op`	`1697264112` B/op	`1697267944` B/op	`1.00`
`BenchmarkTree/20000_vertices_from_protobuf - allocs/op`	`580046` allocs/op	`580043` allocs/op	`1.00`
`BenchmarkTree/30000_vertices_to_protobuf`	`11940889` ns/op 19318329 B/op 210030 allocs/op	`12087022` ns/op 19318328 B/op 210030 allocs/op	`0.99`
`BenchmarkTree/30000_vertices_to_protobuf - ns/op`	`11940889` ns/op	`12087022` ns/op	`0.99`
`BenchmarkTree/30000_vertices_to_protobuf - B/op`	`19318329` B/op	`19318328` B/op	`1.00`
`BenchmarkTree/30000_vertices_to_protobuf - allocs/op`	`210030` allocs/op	`210030` allocs/op	`1`
`BenchmarkTree/30000_vertices_from_protobuf`	`1636363909` ns/op 3752052608 B/op 870052 allocs/op	`1624417393` ns/op 3752053464 B/op 870140 allocs/op	`1.01`
`BenchmarkTree/30000_vertices_from_protobuf - ns/op`	`1636363909` ns/op	`1624417393` ns/op	`1.01`
`BenchmarkTree/30000_vertices_from_protobuf - B/op`	`3752052608` B/op	`3752053464` B/op	`1.00`
`BenchmarkTree/30000_vertices_from_protobuf - allocs/op`	`870052` allocs/op	`870140` allocs/op	`1.00`

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

codecov · 2024-07-22T02:37:56Z

Codecov Report

Attention: Patch coverage is 31.34328% with 46 lines in your changes missing coverage. Please review.

Project coverage is 48.52%. Comparing base (fcf6fbf) to head (62a461e).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
server/server.go	0.00%	13 Missing ⚠️
pkg/document/internal_document.go	0.00%	12 Missing ⚠️
server/packs/packs.go	0.00%	8 Missing ⚠️
server/backend/config.go	0.00%	5 Missing ⚠️
server/config.go	61.53%	5 Missing ⚠️
pkg/document/document.go	0.00%	2 Missing ⚠️
server/packs/pushpull.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #936      +/-   ##
==========================================
- Coverage   48.76%   48.52%   -0.24%     
==========================================
  Files          79       79              
  Lines       11343    11366      +23     
==========================================
- Hits         5531     5515      -16     
- Misses       5269     5309      +40     
+ Partials      543      542       -1

Flag	Coverage Δ
	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a4ce314 and 74bca05.

Files selected for processing (7)

client/client.go (1 hunks)
pkg/document/document.go (2 hunks)
server/clients/clients.go (3 hunks)
server/clients/housekeeping.go (1 hunks)
server/rpc/yorkie_server.go (1 hunks)
server/server.go (1 hunks)
test/integration/presence_test.go (1 hunks)

Additional comments not posted (10)

server/clients/housekeeping.go (1)
72-72: Verify the correctness of passing be instead of be.DB.

Ensure that the Deactivate function is updated to handle the be object correctly and that all necessary properties and methods are accessible.

Verification successful

Verified the correctness of passing be instead of be.DB.

The Deactivate function in server/clients/clients.go is updated to accept the be object, confirming that the change in DeactivateInactives is appropriate.

server/clients/clients.go: func Deactivate(ctx context.Context, be *backend.Backend, refKey types.ClientRefKey) (*database.ClientInfo, error)
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `be` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `be` instead of `be.DB`.
rg --type go -A 5 'func Deactivate(ctx context.Context, be *backend.Backend' 
Length of output: 201

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `be` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `be` instead of `be.DB`.
rg --type go -A 5 'func Deactivate(ctx context.Context, be \*backend.Backend)' 
Length of output: 80

Script:
#!/bin/bash
# Description: Locate the `Deactivate` function and inspect its definition.

# Test: Search for the `Deactivate` function definition in Go files.
rg --type go 'func Deactivate' -A 10
Length of output: 1645
server/clients/clients.go (3)
64-67: Handle potential errors when fetching client information.

Ensure that the error handling logic is robust when fetching client information using FindClientInfoByRefKey.

Verification successful

Handle potential errors when fetching client information.

The error handling logic for FindClientInfoByRefKey is robust, as it consistently returns nil and the error when an error is encountered.

server/clients/clients.go, lines 64-67
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that the error handling logic is robust when fetching client information.

# Test: Search for the `FindClientInfoByRefKey` function calls. Expect: Proper error handling should be in place.
rg --type go -A 5 'FindClientInfoByRefKey' 
Length of output: 9360
101-132: Verify document update and presence clearing logic.

Ensure that the document update and presence clearing logic is correctly implemented and that the PushPull operation is successful.

Line range hint 55-132:
Verify the correctness of passing be instead of db.

Ensure that the Deactivate function handles the be object correctly and that all necessary properties and methods are accessible.
server/server.go (1)
153-153: Verify the correctness of passing r.backend instead of r.backend.DB.

Ensure that the Deactivate function is updated to handle the r.backend object correctly and that all necessary properties and methods are accessible.
Verification successful

Verified the correctness of passing r.backend instead of r.backend.DB.

The Deactivate function in server/clients/clients.go correctly accepts r.backend (of type *backend.Backend), ensuring that all necessary properties and methods are accessible.
server/clients/clients.go:
func Deactivate(
	ctx context.Context,
	be *backend.Backend,
	refKey types.ClientRefKey,
) (*database.ClientInfo, error) {
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `r.backend` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `r.backend` instead of `r.backend.DB`.
rg --type go -A 5 'func Deactivate(ctx context.Context, be *backend.Backend' 
Length of output: 201

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `r.backend` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `r.backend` instead of `r.backend.DB`.
rg --type go -A 5 'func Deactivate(ctx context.Context, be $_'
Length of output: 173

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `r.backend` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `r.backend` instead of `r.backend.DB`.
rg --type go -A 5 'func Deactivate(ctx context.Context, be .*'
Length of output: 173

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `r.backend` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `r.backend` instead of `r.backend.DB`.
rg --type go -A 5 'func Deactivate$ctx context.Context, be .*$'
Length of output: 67

Script:
#!/bin/bash
# Description: Verify that the `Deactivate` function is updated to handle the `r.backend` object correctly.

# Test: Search for the `Deactivate` function definition. Expect: The function should accept `r.backend` instead of `r.backend.DB`.
rg --type go -A 5 'func Deactivate'
Length of output: 755
pkg/document/document.go (2)
140-147: LGTM! Consider adding options parameter.

The ToDocument function correctly initializes the Document struct. To make the function more versatile, consider adding an options parameter.
func ToDocument(internalDocument *InternalDocument) *Document {
	return &Document{
		doc: internalDocument,
+		options: Options{},
	}
}
181-182: Implement logic to revert change if detach request fails.

The comment correctly indicates the need to revert the change if the detach request fails. Consider implementing this logic.
// If the detach request fails, you must revert the
// change before creating the `presenceClear` change.
if err := c.Execute(d.doc.root, d.doc.presences); err != nil {
	// Revert the change here
	return err
}
test/integration/presence_test.go (1)
470-491: LGTM! Consider adding assertions for d1.

The test case correctly validates the presence management logic. To make it more comprehensive, consider adding assertions to verify the presence count of d1 after deactivation.
assert.NoError(t, defaultServer.DeactivateClient(ctx, c1))
assert.NoError(t, c2.Sync(ctx))

+ assert.Equal(t, 1, len(d1.AllPresences()))
assert.Equal(t, 1, len(d2.AllPresences()))
server/rpc/yorkie_server.go (1)

97-97: LGTM! Ensure compatibility with dependent functionalities.

The change to use s.backend instead of s.backend.DB in the DeactivateClient method looks good. Ensure that all dependent functionalities and integrations are compatible with this new parameter structure.

client/client.go (1)

395-396: Implement a rollback mechanism for failed detach requests.

The TODO comment indicates the need for a rollback mechanism if the detach request fails. This is crucial to maintain consistency and integrity. Consider implementing this mechanism to revert any changes made prior to the detach request.

Ensure that the rollback mechanism is implemented and tested thoroughly.

coderabbitai

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

server/clients/clients.go (1)

105-110: Track the TODO task for future improvements.

The TODO comment provides context for future improvements. Consider tracking this task in a project management tool.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 74bca05 and 3409bf1.

Files selected for processing (7)

client/client.go (1 hunks)
pkg/document/document.go (1 hunks)
pkg/document/internal_document.go (2 hunks)
server/clients/clients.go (2 hunks)
server/documents/documents.go (4 hunks)
server/packs/packs.go (2 hunks)
server/packs/pushpull.go (1 hunks)

Files skipped from review as they are similar to previous changes (1)

client/client.go

Additional comments not posted (22)

server/clients/clients.go (11)

25-28: LGTM!

The new imports are necessary for the updated logic in the Deactivate function.

Also applies to: 30-30

54-56: LGTM!

The function signature update to use *backend.Backend allows for more intricate interactions with the backend.

57-66: Ensure proper error handling for client information retrieval.

The error handling for retrieving client information is appropriate. Ensure that similar error handling is applied consistently throughout the function.

68-72: Ensure proper error handling for actor ID conversion.

The error handling for converting client ID to actor ID is appropriate. Ensure that similar error handling is applied consistently throughout the function.

74-78: Ensure proper error handling for project information retrieval.

The error handling for retrieving project information is appropriate. Ensure that similar error handling is applied consistently throughout the function.

80-84: Ensure document detachment logic handles all attached documents.

The loop correctly iterates over all documents associated with the client. Ensure that the condition info.Status != database.DocumentAttached accurately identifies documents that need to be detached.

85-92: Ensure proper error handling for document information retrieval.

The error handling for retrieving document information is appropriate. Ensure that similar error handling is applied consistently throughout the function.

93-96: Ensure proper error handling for building document for checkpoint.

The error handling for building the document for the checkpoint is appropriate. Ensure that similar error handling is applied consistently throughout the function.

98-103: Ensure proper error handling for document state update.

The error handling for updating the document state by clearing the client's presence is appropriate. Ensure that similar error handling is applied consistently throughout the function.

105-114: Ensure proper error handling for push-pull operation.

The error handling for the push-pull operation is appropriate. Ensure that similar error handling is applied consistently throughout the function.

119-123: Ensure proper error handling for client deactivation.

The error handling for deactivating the client is appropriate. Ensure that similar error handling is applied consistently throughout the function.

server/packs/pushpull.go (1)

142-142: LGTM! Verify the behavior of the new function.

The change to use BuildInternalDocForServerSeq suggests a shift in the logic or source of the document being constructed. Ensure that the new function behaves as expected.

server/documents/documents.go (4)

74-74: LGTM! Verify the behavior of the new function.

The change to use BuildInternalDocForServerSeq suggests a shift in the logic or source of the document being constructed. Ensure that the new function behaves as expected.

105-105: LGTM! Verify the behavior of the new function.

The change to use BuildInternalDocForServerSeq suggests a shift in the logic or source of the document being constructed. Ensure that the new function behaves as expected.

138-138: LGTM! Verify the behavior of the new function.

The change to use BuildInternalDocForServerSeq suggests a shift in the logic or source of the document being constructed. Ensure that the new function behaves as expected.

182-182: LGTM! Verify the behavior of the new function.

The change to use BuildInternalDocForServerSeq suggests a shift in the logic or source of the document being constructed. Ensure that the new function behaves as expected.
server/packs/packs.go (3)
33-33: Import statement approved.

The import for github.com/yorkie-team/yorkie/pkg/document/time is necessary for handling the actorID parameter in the updated BuildDocForCheckpoint function.

233-233: Function repositioning approved.

The function BuildInternalDocForServerSeq has been moved to follow BuildDocForCheckpoint, suggesting a logical grouping of related functionalities.

214-231: Function changes approved.

The function BuildDocumentForServerSeq has been renamed to BuildDocForCheckpoint with additional parameters clientSeq and actorID. These changes enhance the function's capability to manage document states in relation to checkpoints.

Ensure that all calls to this function match the new signature.

Verification successful

Verification successful.

All instances of the function BuildDocForCheckpoint in the codebase match the new signature.

server/clients/clients.go
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `BuildDocForCheckpoint` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'BuildDocForCheckpoint'
Length of output: 785

Script:
#!/bin/bash
# Description: Verify all function calls to `BuildDocForCheckpoint` match the new signature.

# Test: Search for the function usage across the entire codebase. Expect: Only occurrences of the new signature.
rg --type go -A 5 'BuildDocForCheckpoint'
Length of output: 785
pkg/document/internal_document.go (2)

139-149: Method changes approved.

The method SyncCheckpoint synchronizes the checkpoint and change ID with the specified server and client sequences. The logic appears correct and aligns with the intended functionality.

388-393: Method changes approved.

The method ToDocument converts the InternalDocument instance into a Document object. The logic appears correct and aligns with the intended functionality.

pkg/document/document.go (1)

457-461: Method changes approved.

The method setInternalDoc allows setting an internal document reference within an instance of Document. The logic appears correct and aligns with the intended functionality.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3409bf1 and e4ded21.

Files selected for processing (3)

server/backend/database/memory/database.go (1 hunks)
server/backend/database/mongo/client.go (1 hunks)
server/backend/database/testcases/testcases.go (1 hunks)

Files skipped from review due to trivial changes (1)

server/backend/database/testcases/testcases.go

Additional comments not posted (2)

server/backend/database/memory/database.go (1)
Line range hint 523-537:
Verify presence clearing logic after detaching documents.

The removal of the loop that detached documents simplifies the control flow. However, ensure that the presence clearing logic is correctly handled elsewhere to avoid potential issues with attached documents.

Verification successful

Verify presence clearing logic after detaching documents.

The presence clearing logic is correctly handled in the codebase. The ApplyChanges method in InternalDocument manages the PresenceChange with a Clear type, ensuring that users are removed from online clients and the 'unwatched' event is triggered. The removal of the loop in the DeactivateClient method does not introduce any issues.

pkg/document/internal_document.go: ApplyChanges method handles PresenceChange with Clear type.

pkg/document/innerpresence/presence.go: Defines PresenceChange and Clear type.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify presence clearing logic after detaching documents.

# Test: Search for presence clearing logic in the codebase. Expect: Appropriate presence clearing logic should be present.
rg --type go -A 5 'PresenceChange{type: .clear.}'
Length of output: 178

Script:
#!/bin/bash
# Description: Verify presence clearing logic after detaching documents.

# Test: Search for presence clearing logic in the codebase. Expect: Appropriate presence clearing logic should be present.
rg --type go -A 5 'PresenceChange{type: "clear"}'
Length of output: 178

Script:
#!/bin/bash
# Description: Verify presence clearing logic after detaching documents.

# Test: Search for presence clearing logic in the codebase. Expect: Appropriate presence clearing logic should be present.
rg --type go -A 5 'PresenceChange' | rg 'clear'
Length of output: 315

Script:
#!/bin/bash
# Description: Extract relevant code to verify presence clearing logic.

# Extract code from pkg/document/internal_document.go
rg --type go -A 10 -B 5 'PresenceChange' pkg/document/internal_document.go

# Extract code from pkg/document/innerpresence/presence.go
rg --type go -A 10 -B 5 'PresenceChange' pkg/document/innerpresence/presence.go
Length of output: 2585
server/backend/database/mongo/client.go (1)

Line range hint 523-537:
Verify document status management after client deactivation.

The removal of the document status update logic simplifies the method. However, ensure that the document status management is correctly handled elsewhere to avoid potential issues with document statuses.

coderabbitai

Actionable comments posted: 3

Outside diff range, codebase verification and nitpick comments (1)

server/backend/database/client_info.go (1)
287-287: Consider adding a function comment.

Adding a comment to describe the purpose and usage of this function would improve code readability and maintainability.
+// ToClient creates a new client connection using the provided RPC address and project key.
+// It sets the client to active status and assigns an actor ID derived from the ClientInfo instance's ID.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e4ded21 and 13211b1.

Files selected for processing (6)

client/client.go (2 hunks)
server/backend/database/client_info.go (2 hunks)
server/clients/clients.go (2 hunks)
server/clients/housekeeping.go (2 hunks)
server/rpc/yorkie_server.go (1 hunks)
server/server.go (3 hunks)

Files skipped from review as they are similar to previous changes (5)

client/client.go
server/clients/clients.go
server/clients/housekeeping.go
server/rpc/yorkie_server.go
server/server.go

Additional comments not posted (1)

server/backend/database/client_info.go (1)
305-305: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to ToClient are correctly implemented.

Verification successful

The ToClient function is defined in server/backend/database/client_info.go and is used in server/clients/clients.go.

To ensure that the function usage is correctly implemented, we need to inspect the relevant lines in server/clients/clients.go for proper error handling and subsequent logic.

Function usage of ToClient is correctly implemented.

The function ToClient is used in server/clients/clients.go with proper error handling and subsequent logic. The implementation aligns with typical Go error handling practices and ensures that the code proceeds correctly only if no error occurs.

server/clients/clients.go: Lines around the usage of ToClient show proper error handling and logical flow.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `ToClient` are correctly implemented.

# Test: Search for the function usage. Expect: Correct implementation of the new function.
rg --type go -A 5 $'ToClient'
Length of output: 873

Script:
#!/bin/bash
# Description: Inspect the relevant lines in `server/clients/clients.go` to ensure the `ToClient` function is used correctly.

# Display the relevant lines around the usage of `ToClient` in `server/clients/clients.go`
rg --type go -A 20 $'clientInfo.ToClient' server/clients/clients.go
Length of output: 642

server/backend/database/client_info.go

…nt from clientInfo

This commit addresses the issue of 'updated_at' being modified even when document content remains unchanged. For now, 'updated_at' updates only when there are operations in changes. All changes are still recorded in DB for client-server consistency, but mere presence updates don't alter the timestamp. This helps reduce unnecessary server load, especially in applications like whiteboards.

The time complexity of creating crdt.TreeNode is O(N^2), potentially causing performance bottlenecks. It's optimized to O(n). While this may not be a significant issue currently, there is a risk that as the number of tree nodes in the protobuf increases, operations will scale quadratically, potentially causing performance bottlenecks. --------- Co-authored-by: JiHwan Yim <raararaara@gmail.com> Co-authored-by: Youngteac Hong <susukang98@gmail.com>

Fix the mongodb query keys slice to return an empty slice if it is empty. Currently, FindDocInfosByKeys throws a query error when passing an empty slice of keys. --------- Co-authored-by: Youngteac Hong <susukang98@gmail.com>

Add the admin APIs ChangePassword and DeleteAccount to enhance the functionality and provide basic account actions for administrators themselves.

Update docker compose command to V2 in accordance of deprecation of docker-compose V1 on GitHub Actions runner's ubuntu image.

If an unhashable error is given, the error cannot be converted to a ConnectError with a map, causing a panic and shutting down the server. --------- Co-authored-by: Youngteac Hong <susukang98@gmail.com>

Added the handler to allow health checks to be performed with plain HTTP GET requests needed for traditional uptime checker or load balancer, along with existing gRPC health check.

This commit adds the functionality to display the version of connected Yorkie server in the version command. The version information can be viewed by converting it into yaml or json format using the --output, -o flag. This flag accepts json and yaml strings and converts them to the appropriate format for display. In order to solely check the version of the client CLI without considering the server's version, the --client flag has been included. The development of this feature was inspired by examining the kubectl.

Change health check endpoint to include yorkie service endpoint along with HEAD method to support various health checkers.

Revised version of fine-tuned CI workflows in PR #964, which is actions/checkout for dorny/paths-filter. The CI failed in main branch, due to dorny/paths-filter action's behavior. It does not require checkout when triggered by PR, but it requires checkout on other triggers. --------- Co-authored-by: binary_ho <dfghcvb11@naver.com>

This PR adds a metric to collect the number of background routines. Prometheus provides its own thread-safe Gauge metric, which can be used to accurately collect the number of concurrent goroutines. Also, although the only task running in the background so far is PushPull, this commit implements the AttachGoroutine() method to take a string called taskType as a parameter, since it seems to be focused on reusability.

Co-authored-by: Youngteac Hong <susukang98@gmail.com>

This commit ensures that when a new Document is created and attached, the updated_at field is properly initialized with the created_at. Prior to commit b494fa2, the updated_at field was correctly set during the attachment process through a push-pull operation that involved presence. However, after that commit, the updated_at field is no longer updated when a document is attached, resulting in inconsistencies.

A. Total Tests: 49 cases(7*7) Initially, the symmetric relationship between clients in operation combinations was not included. However, since the Lamport clock comparison varies depending on the client ID, the tests were modified to check all cases. B. Operations per Client: 7(2+3+1+1) - Insert: 2 cases(Prev, Prev.Next) - Move: 3 cases(Prev, Prev.Next, Target) - Set: 1 case(Target) - Remove: 1 case(Target) C. Failure Cases: 10 cases(2*2 + 2 + 1*2 + 1*2) 1. *.Prev == Move.Target 4 cases(2*2, symmetric) - Insert.Prev == Move.Target - Move.Prev == Move.Target 2. *.Prev.Next == Set.Target: 2 cases(asymmetric) - Insert.Prev.Next == Set.Target - Move.Prev.Next == Set.Target 3. Move.Target == Set.Target: 2 cases(1*2, symmetric, client ID matters) 4. Remove.Target == Set.Target: 2 cases (1*2, symmetric)

#974) This commit modifies the FindChangeInfosBetweenServerSeqs method to avoid executing DB queries when the from > to. This change minimizes unnecessary database resource consumption, particularly in scenarios where the most recent document editor, User A, continues editing without any interference from other users (B, C, etc.). In such cases, in PushPull, User A's Checkpoint.serverSeq always equal to the server's initialServerSeq (DocInfo.serverSeq), leading to a situation where from > to in the FindChangeInfosBetweenServerSeqs. By preventing the execution of a query under these circumstances, we can reduce the load on database resources.

Related to #985

Added functionality allowing users to delete accounts and change passwords through the CLI to support recent development of admin ChangePassword and DeleteAccount APIs on the server side.

hackerwins · 2024-10-16T10:07:11Z

This PR continues in #1036.

raararaara added 2 commits July 19, 2024 18:03

Modify clients.Deactivate to handle presence clear

5d48edb

Resolve conflict

fb631fa

hackerwins added 2 commits July 22, 2024 11:29

Fix invalid argument passing

90cfe15

Fix lint warning

e496ad2

github-actions bot reviewed Jul 22, 2024

View reviewed changes

raararaara added 9 commits July 22, 2024 14:34

Add related test

a87a9f1

Add logic to prevent duplicate presence clear change

1da3ec3

Simplify presenceChange clear condition

525d4c4

Add test related to duplicate presence clear

28b0db5

Lint

1c5b05c

Fix test to return correct result

e0b9fce

Add additional comments

597b867

Cleanup code

e39b542

Cleanup code

74bca05

raararaara marked this pull request as ready for review July 23, 2024 09:25

raararaara requested a review from hackerwins July 23, 2024 09:26

coderabbitai bot reviewed Jul 23, 2024

View reviewed changes

Revise the deactivation logic

3409bf1

coderabbitai bot reviewed Jul 23, 2024

View reviewed changes

Revert queries and test related to DeactivateClient

e4ded21

coderabbitai bot reviewed Jul 24, 2024

View reviewed changes

Modify clients.Deactivate to load client within the method

13211b1

coderabbitai bot reviewed Jul 26, 2024

View reviewed changes

server/backend/database/client_info.go Outdated Show resolved Hide resolved

server/backend/database/client_info.go Outdated Show resolved Hide resolved

server/backend/database/client_info.go Outdated Show resolved Hide resolved

raararaara marked this pull request as draft July 26, 2024 06:10

raararaara added 4 commits July 26, 2024 17:12

Fix rpcAddr to cover other testcases

cc14a6c

Lint

ab278e7

Fix ToClient to allow token value to be received when creating clie…

17de638

…nt from clientInfo

Lint

532f87d

raararaara and others added 28 commits October 7, 2024 17:35

Cleanup code

669d3ef

Update CHANGELOG.md for v0.4.28 (#942)

1791d7f

Fix FindDocInfosByKeys when keys is empty (#945)

b3f0362

Fix the mongodb query keys slice to return an empty slice if it is empty. Currently, FindDocInfosByKeys throws a query error when passing an empty slice of keys. --------- Co-authored-by: Youngteac Hong <susukang98@gmail.com>

Support basic account action for admin (#934)

cee8d9a

Add the admin APIs ChangePassword and DeleteAccount to enhance the functionality and provide basic account actions for administrators themselves.

Update docker compose command to V2 (#950)

49bc08c

Update docker compose command to V2 in accordance of deprecation of docker-compose V1 on GitHub Actions runner's ubuntu image.

Handle panic during conversion to connectCode (#951)

8ea4fa2

If an unhashable error is given, the error cannot be converted to a ConnectError with a map, causing a panic and shutting down the server. --------- Co-authored-by: Youngteac Hong <susukang98@gmail.com>

Update CHANGELOG.md for v0.4.29 (#953)

e76544a

Add HTTP health check handler for server health monitoring (#952)

39929ec

Added the handler to allow health checks to be performed with plain HTTP GET requests needed for traditional uptime checker or load balancer, along with existing gRPC health check.

Update README.md

c8c744d

Update CHANGELOG.md for v0.4.30 (#956)

7d189b6

Modify health check endpoint and add HEAD method (#958)

9a21c0c

Change health check endpoint to include yorkie service endpoint along with HEAD method to support various health checkers.

Fix invalid test case in FindDocInfosByKeys (#972)

f8f40fc

Co-authored-by: Youngteac Hong <susukang98@gmail.com>

Update CHANGELOG.md for v0.4.31 (#973)

b254db1

Rename SetByIndex to ArraySet (#995)

ad8161c

Related to #985

Add Account Deletion and Change Password to CLI Commands (#983)

43a4340

Added functionality allowing users to delete accounts and change passwords through the CLI to support recent development of admin ChangePassword and DeleteAccount APIs on the server side.

Merge branch 'main' of https://github.com/yorkie-team/yorkie

10b87cf

Merge branch 'main' into housekeeping-remove-presence

c8ad4ab

Add missing param

e7221c9

Add system server configuration to ensure execute main test properly

b6eba52

Add system server configuration to ensure execute main test properly

62a461e

hackerwins closed this Oct 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear presence when housekeeping process #936

Clear presence when housekeeping process #936

raararaara commented Jul 22, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 22, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

github-actions bot left a comment •

edited

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

hackerwins commented Oct 16, 2024

Clear presence when housekeeping process #936

Clear presence when housekeeping process #936

Conversation

raararaara commented Jul 22, 2024 • edited by coderabbitai bot Loading

What this PR does / why we need it?

Any background context you want to provide?

What are the relevant tickets?

Summary by CodeRabbit

Summary by CodeRabbit

coderabbitai bot commented Jul 22, 2024 • edited Loading

Review skipped

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

github-actions bot left a comment • edited Loading

Choose a reason for hiding this comment

Go Benchmark

codecov bot commented Jul 22, 2024 • edited Loading

Codecov Report

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

hackerwins commented Oct 16, 2024

raararaara commented Jul 22, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 22, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

github-actions bot left a comment •

edited

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading