Skip to content

Conversation

Chriztiaan
Copy link
Contributor

@Chriztiaan Chriztiaan commented Feb 7, 2025

This allows the fetch strategy to be configured, with the actual functionality addition being the sequential option.

When specifying sequential:
Instead of queueing up multiple sync events (10 originally) before applying them, which would lead to less round-trips - we are configuring the queue size to 1, meaning each event will get processed individually before asking for the next.
This results in less overhead processing of the netword events (pending items) and should free up the web socket for better keep alive acknowledgements.

Currently this resolves a known issue on slower devices with large sync datasets.

@changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2025

🦋 Changeset detected

Latest commit: 93615f3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@powersync/react-native Minor
@powersync/common Minor
@powersync/op-sqlite Patch
@powersync/tanstack-react-query Patch
@powersync/web Patch
@powersync/diagnostics-app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…either `buffered` or `sequential` for the Websocket connect option.
@Chriztiaan Chriztiaan changed the title Test slower sockets feat: Add FetchStrategy option to connect() for WebSockets Feb 7, 2025
Copy link
Contributor

@rkistner rkistner left a comment

Choose a reason for hiding this comment

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

Do you know what is specifically causing the issue?

For example, is the bson stream parsing adding so much overhead on a slow react-native device that we can't even send keepalive packet? If that's the case, could we do something like throttle the bson parsing to avoid that?

Or is there a more fundamental issue with having a queue backlog?

In general, it would be great if we could auto-manage this, without the developer having to figure out which option to use for their use case.

@Chriztiaan Chriztiaan marked this pull request as ready for review February 11, 2025 06:41
@Chriztiaan Chriztiaan merged commit 893d42b into main Feb 11, 2025
6 checks passed
@Chriztiaan Chriztiaan deleted the test-slower-sockets branch February 11, 2025 07:53
@rkistner rkistner mentioned this pull request Jun 30, 2025
1 task
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