Skip to content

Conversation

@ubik2
Copy link
Contributor

@ubik2 ubik2 commented Jul 30, 2025

Previously, when we pushed an item into an array, we would write the doc with the array, then later, write the doc with the array element.
This is problematic for another client, since they can get the updated array, and then attempt to load the array element before it has been written to the server, which means they end up treating it as an empty element.
This PR changes things to batch the write of a document to include any linked documents reachable by the schema query.

There are some problems with this code, but it makes things better.


Summary by cubic

Fixed an issue where pushing an item to an array could cause clients to see empty elements by batching document writes to include all linked documents.

  • Bug Fixes
    • Ensured that when an array is updated, all related linked documents are written together to prevent clients from loading incomplete data.

// We may have linked docs that storage doesn't know about
const storageProvider = this._getStorageProviderForSpace(doc.space);
const { missing } = this._queryLocal(doc.space, uri, storageProvider);
const { missing, loaded } = this._queryLocal(
Copy link
Contributor Author

@ubik2 ubik2 Jul 30, 2025

Choose a reason for hiding this comment

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

By doing the queryLocal here, before the await runtime.idle in sendDocValue, it's possible that the set of linked documents could change between this check and the value capture in sendDocValue, meaning that we may link to different docs by the time we're in there.
This PR is mostly a stopgap, though, since a transaction should contain better information about linked docs.
As mentioned below, there's also an issue with the timing of pulling cells we are linked to, but don't already have.
This can happen if you change the schema of a cell to include docs that were previously excluded. It can also occur if you have a well known cause that you used to create a cell (so you have a local unsynced copy and there's also a copy on the server).

@ubik2 ubik2 requested a review from seefeldb July 30, 2025 19:40
@ubik2
Copy link
Contributor Author

ubik2 commented Jul 30, 2025

I may also add the modified array_push integration test, since it's different enough from the main test.

@seefeldb seefeldb merged commit 2a920ce into main Jul 30, 2025
9 checks passed
@seefeldb seefeldb deleted the fix/array-push-send-linked branch July 30, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants