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

[Feature] Add warning if crud transactions are not completed #254

Merged
merged 4 commits into from
Aug 6, 2024

Conversation

stevensJourney
Copy link
Collaborator

@stevensJourney stevensJourney commented Aug 1, 2024

Overview

Currently when processing CRUD items the PowerSyncDatabase client will call the backend connector's uploadData function whenever CrudEntry items are present in the ps_crud internal table.

The uploadData callback has access to the PowerSyncDatabase client which it can use to get the pending CrudEntrys. The connector can consume the CRUD events in numerous different ways.

Typically the uploadData callback will either call getNextCrudTransaction or getCrudBatch on the PowerSyncDatabase.

  • getNextCrudTransaction will return the next (Single) group of CrudEntrys which occurred in the same transaction as an instance of CrudTransaction. If an operation did not occur in a transaction then only a single operation will be returned.
  • getCrudBatch returns n CrudEntrys as an instance of CrudBatch . These entires are not grouped by transaction.

The CrudBatch and CrudTransaction classes each have a complete method which will remove the CrudEntrys from the CRUD queue. This should be executed in the uploadData method after the items have successfully been uploaded.

Given the nature above, there may be more items in the queue than what was consumed by a single invocation of the uploadData method. In this case the SDK will continuously check if CrudEntrys are present in the queue and call uploadData until the queue is empty.

Issues can occur if the uploadData callback fetches CRUD items and uploads them without calling and awaiting the CrudTransaction or CrudBatch complete method.

Failing to await a complete() call in the uploadData method is mostly guarded in the SDK due to limits on concurrent SQLite operations. Previously the SqliteBucketStorage implementation used .execute methods for checking if CRUD items were available. Internal queues would (luckily) ensure the next upload iteration would essentially await for the complete action to complete.

Failing to call the complete() method altogether is a much worse scenario. This would result in CrudEntrys not being removed from the queue (after they have been uploaded). The SDK previously would blindly check if CrudEntrys were still present and immediately call uploadData to upload them - this will result in an infinite loop of repeated uploads.

This PR replaces the use of the hasCrud check with getting the CrudEntry which is first in line to be uploaded (the behaviour of hasCrud is matched if the entry exists). The SDK can then keep track of this CrudEntry in the upload loop. If it sees the same entry that it already processed previously a warning will be printed and the upload loop will be delayed in order to slow down the infinite loop. This does not currently prevent the loop from continuing.

This should help for issues such as #247.

Copy link

changeset-bot bot commented Aug 1, 2024

🦋 Changeset detected

Latest commit: 8a87ec2

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

This PR includes changesets to release 4 packages
Name Type
@powersync/common Minor
@powersync/web Minor
@powersync/react-native Minor
@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

@stevensJourney stevensJourney marked this pull request as ready for review August 6, 2024 09:59
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.

4 participants