Sync fails with IndexedDB due to weird Transaction semantics

[Slug-Man]

When calling replica.sync() with IndexedDbStorage in a browser, sync fails with (in Chrome): InvalidStateError: Failed to execute 'objectStore' on 'IDBTransaction': The transaction has finished.

Root Cause:
The sync code in taskdb/sync.rs opens a single transaction and holds it open while:

1. Reading from storage (IDB operations)
2. Making HTTP calls to the sync server (network I/O)
3. Writing back to storage (IDB operations)

IndexedDB transactions auto-commit when there are no pending IDB operations. During HTTP calls, the browser sees no pending IDB work and closes the transaction. When sync tries to write afterward, it fails.

[djmitche]

Thanks! I did miss that important detail in implementing this functionality, and it's a pretty fundamental flaw in the WASM support.

I suppose we have two options:

• Somehow make the Txn implementation satisfy the requirements of that trait -- perhaps by artificially holding the transaction open with repeated operations, or by layering on some kind of in-database locking like an is_locked table.
• Re-writing the sync algorithm so that it does not require the transaction to be open when contacting the remote server.

The first is impractical: busy-looping with repeated operations would waste a lot of time and energy, and a simple lock doesn't accomplish the not-visible-until-committed requirement.

The second might be possible. There are two remote calls involved in sync: GetChildVersion and AddVersion. In both cases, we are already able to "crash" either before or after the server receives the request without data corruption. This implies that DB changes may occur while the API call proceeds: API call begins, process crashes, another process starts and makes changes. So, the questions is, do we rely on transaction rollback at any point -- that is, is there a point where something has been modified in the DB that must be rolled back if the response from the server is not as expected.

GetChildVersion is called having written nothing, so this is pretty easy. It depends on base_version, and could easily re-validate that value after the API call.

AddVersion takes a version, which we construct from the DB, but nothing is written to the DB before the API call. However, it does depend on the operations in that version not changing during the API call. We change operations by marking them as sync'd -- which would be OK, as it would result in duplicating operations on the server and this is already possible and handled properly.

In summary, it should be possible to re-write the sync process such that it works with shorter transactions that do not span over API calls.

[djmitche]

I can reproduce the issue with tests now.

The batching and storing transformed_server_ops make the fix a bit harder than I expected!