CLAassistant
commented
3 months ago
All committers have signed the CLA.
Closed window9u closed 3 months ago
CLAassistant
commented
3 months ago
All committers have signed the CLA.
coderabbitai[bot]
commented
3 months ago The changes introduce enhanced logic for updating the UpdatedAt timestamp in the database's document handling. Specifically, the UpdatedAt field is now only updated when relevant operations are present, ensuring more accurate tracking of document modifications. This refinement reduces unnecessary updates and aligns the system's behavior with the intended functionality, particularly regarding changes in the document's root content.
| File | Change Summary |
|---|---|
server/backend/database/memory/database.go |
Modified CreateChangeInfos to update UpdatedAt only when operations are present in changes. |
server/backend/database/mongo/client.go |
Adjusted CreateChangeInfos to conditionally update updated_at in the updateFields map based on operations. |
server/backend/database/testcases/testcases.go |
Added a test case verifying UpdatedAt behavior when only presence or both presence and operations are changed. |
sequenceDiagram
participant Client
participant Database
Client->>Database: Update Document Presence
Database->>Database: Check Operations
alt No Operations
Database-->>Client: UpdatedAt remains unchanged
else Operations present
Database-->>Database: Update UpdatedAt to now
Database-->>Client: UpdatedAt modified
end| Objective | Addressed | Explanation |
|---|---|---|
| Update Document updatedAt to only include Document.Root modifications (#916) | ✅ | |
| Avoid unnecessary updates to Document's updatedAt when no content changes (#916) | ✅ |
🐰 In the garden where changes bloom,
A timestamp's dance dispels the gloom.
Only when roots find their way,
Updated moments greet the day.
Hooray for logic, bright and clear,
For every change we hold so dear! 🌼
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?
What this PR does / why we need it:
The current implementation updates the 'updated_at' field of a document even if the content itself remains unchanged. This happens because changes reported by the client may contain both "presence" and "operations." With the changes introduced in this commit, the 'updated_at' timestamp will only be updated when there is more than one operation in the change list.
Initially, I attempted to address this issue by avoiding database operations when the changes were limited to presence updates. However, this approach was unsuccessful for the following reasons:
Consequently, it is necessary to store all changes in the database, even if they do not modify the core content, to maintain consistency.
In the context of applications like a whiteboard, where numerous operations can occur simply from moving a mouse pointer, this issue becomes more pronounced. Such frequent minor updates lead to a high volume of changes that primarily involve presence, inflating the ClientSeq and triggering server snapshots, which add to the server's load.
Proposed Solution
Initially, I considered separating presence data from document updates and managing it through an in-memory database like Redis. This would involve removing presence data from the push-pull synchronization mechanism and introducing a new API dedicated to presence management. However, this approach would require extensive modifications to the existing codebase and protocols, affecting not just the current system but other SDKs as well.
Given the complexities involved, I recommend keeping the current push-pull mechanism unchanged for stability and introducing a new API dedicated to broadcasting user presence. This strategy is ideal for applications where excessive updates are driven primarily by presence changes. Developers can shift presence management from the standard update method to this new broadcast API.
This broadcast API is specifically designed to manage presence updates independently. It broadcasts changes to all document subscribers efficiently without modifying the sequence number (ClientSeq) or permanently storing the updates. This design ensures optimal performance by minimizing unnecessary data load and sequence adjustments, making it highly suitable for managing frequent, low-impact changes.
Which issue(s) this PR fixes:
Fixes #916
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
UpdatedAttimestamp, now only updating when there are actual operations present.Tests
UpdatedAttimestamp based on document updates and operations.Bug Fixes
UpdatedAtfield is updated, ensuring more accurate timestamp management.