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.

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

Introduce MongoDB sharding rules to Documents and Document-wide collections #734

Merged
merged 9 commits into from
Jan 19, 2024

Conversation

sejongk
Copy link
Contributor

@sejongk sejongk commented Dec 20, 2023

What this PR does / why we need it:
This PR introduces MongoDB sharding rules to documents and document-wide collections (e.g. Changes, SyncedSync, Snapshots).

Which issue(s) this PR fixes:

Addresses #673

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

@sejongk sejongk added the protocol changed 📝 Whether the protocol has changed label Dec 20, 2023
Copy link

codecov bot commented Dec 20, 2023

Codecov Report

Attention: 183 lines in your changes are missing coverage. Please review.

Comparison is base (eaefcdc) 50.17% compared to head (46f7010) 50.87%.
Report is 1 commits behind head on main.

Files Patch % Lines
server/backend/database/mongo/client.go 64.70% 48 Missing ⚠️
server/backend/database/memory/database.go 52.63% 26 Missing and 1 partial ⚠️
server/backend/database/mongo/registry.go 65.78% 18 Missing and 8 partials ⚠️
server/rpc/yorkie_server.go 69.23% 16 Missing ⚠️
server/backend/database/client_info.go 71.73% 13 Missing ⚠️
server/backend/sync/memory/pubsub.go 45.83% 13 Missing ⚠️
server/rpc/admin_server.go 52.17% 11 Missing ⚠️
server/backend/database/snapshot_info.go 0.00% 9 Missing ⚠️
client/client.go 0.00% 8 Missing ⚠️
api/types/resource_ref_key.go 57.14% 6 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #734      +/-   ##
==========================================
+ Coverage   50.17%   50.87%   +0.70%     
==========================================
  Files          69       70       +1     
  Lines       10165    10168       +3     
==========================================
+ Hits         5100     5173      +73     
+ Misses       4526     4473      -53     
+ Partials      539      522      -17     

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

@sejongk sejongk force-pushed the add-sharding-rule-to-mongodb2 branch from 72694b0 to f20f0fa Compare December 20, 2023 08:07
@sejongk sejongk force-pushed the add-sharding-rule-to-mongodb2 branch from f20f0fa to 2bc01f9 Compare December 20, 2023 08:08
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

I looked at the code up to the memory DB. and I have a question.

MemoryDB runs standalone, and it is not sharded based on shard keys. If we receive refKey due to change in interface, we only need to utilize the id. So there's no necessity to use the key in the implementation. What are your thoughts on this?

server/backend/database/memory/indexes.go Outdated Show resolved Hide resolved
server/backend/database/memory/database.go Outdated Show resolved Hide resolved
server/backend/database/memory/database.go Outdated Show resolved Hide resolved
@sejongk
Copy link
Contributor Author

sejongk commented Dec 21, 2023

Thanks for your contribution.

I looked at the code up to the memory DB. and I have a question.

MemoryDB runs standalone, and it is not sharded based on shard keys. If we receive refKey due to change in interface, we only need to utilize the id. So there's no necessity to use the key in the implementation. What are your thoughts on this?

I think that's also a good idea.
Previously, I heard that the MemoryDB is utilized for the testing purpose, so I tried to diminish the gap between the implementation of MongoDB and MemoryDB for the testing consistency.

test/helper/helper.go Outdated Show resolved Hide resolved
@sejongk sejongk marked this pull request as draft January 18, 2024 05:59
@sejongk sejongk marked this pull request as ready for review January 18, 2024 06:19
@sejongk sejongk changed the title Introduce sharding rules to MongoDB collections Introduce MongoDB sharding rules to Documents and Document-wide collections Jan 18, 2024
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@hackerwins hackerwins merged commit 6651d42 into main Jan 19, 2024
4 checks passed
@hackerwins hackerwins deleted the add-sharding-rule-to-mongodb2 branch January 19, 2024 10:59
highcloud100 pushed a commit that referenced this pull request Jan 26, 2024
This commit introduces MongoDB sharding rules to documents and
document-wide collections (e.g. Changes, SyncedSync, Snapshots).

Changes:
- Introduce RefKey to represent ID and shard key together instead of ID.
- Change the reference key of Users from _id to username.
- Introduce encoders for types.ID, time.ActorID and ClientDocInfo to MongoDB.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol changed 📝 Whether the protocol has changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants