amplify-js: DataStore latter updates should take precedent on a slow network
Describe the bug Say you have a button to update a Post title. With that you change the post title to ‘Hairy Potter’. This mutation is queued in the outbox. However before this mutation is processed, you decides to update the post title again to ‘Marry in the Soberland’. As a user, you would expect your post title to be permanently change to the latter title as the local updates reflects that. Once the both mutations are processed, the post title would return to ‘Hairy Potter’ which isn’t correct.
To Reproduce
- On iOS, enable Network Link Conditioner, choose Edge(or even worse) as the profile.
- Update a post title to ‘Title A’.
- Before the mutation is processed, quickly update the post title to ‘Title B’.
- After both mutations are done, you would notice the post title is back to ‘Title A’.
Expected behavior The expected post title should be ‘Title B’.
What is Configured? We are using ‘Auto merge’ as the Conflict Handler, however it shouldn’t matter for this case. As I would expect DataStore to give precedent to the second mutation because the first mutation is not processed yet. Personally, I think the solution would be to increment the version number by 1 to the second mutation after knowing that the first mutation is processed.
Environment
System:
OS: macOS 10.15.5
CPU: (4) x64 Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
Memory: 59.48 MB / 8.00 GB
Shell: 5.7.1 - /bin/zsh
Binaries:
Node: 14.3.0 - /usr/local/bin/node
Yarn: 1.22.4 - /usr/local/bin/yarn
npm: 6.14.4 - /usr/local/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
Browsers:
Chrome: 83.0.4103.116
Safari: 13.1.1
npmPackages:
@babel/core: ^7.9.0 => 7.10.3
@babel/runtime: ^7.9.2 => 7.10.3
@react-native-community/async-storage: ^1.11.0 => 1.11.0
@react-native-community/eslint-config: ^1.0.0 => 1.1.0
@react-native-community/masked-view: ^0.1.9 => 0.1.10
@react-native-community/netinfo: ^5.7.1 => 5.9.4
@react-navigation/drawer: ^5.4.1 => 5.8.4
@react-navigation/native: ^5.1.5 => 5.6.1
@react-navigation/stack: ^5.2.10 => 5.6.2
aws-amplify: ^3.0.9 => 3.0.20
aws-amplify-react-native: ^4.0.4 => 4.2.1
babel-jest: ^25.3.0 => 25.5.1
babel-plugin-root-import: ^6.5.0 => 6.5.0
eslint: ^6.8.0 => 6.8.0
eslint-import-resolver-babel-plugin-root-import: ^1.1.1 => 1.1.1
immutability-helper: ^3.0.2 => 3.1.1
jest: ^25.3.0 => 25.5.4
metro-react-native-babel-preset: ^0.59.0 => 0.59.0
moment: ^2.24.0 => 2.27.0
patch-package: ^6.2.2 => 6.2.2
postinstall-postinstall: ^2.1.0 => 2.1.0
prettier: ^2.0.5 => 2.0.5
react: 16.11.0 => 16.11.0
react-devtools: ^4.6.0 => 4.7.0
react-native: ^0.62.2 => 0.62.2
react-native-barcode-mask: ^1.2.2 => 1.2.4
react-native-camera: ^3.23.0 => 3.30.0
react-native-device-info: ^5.6.1 => 5.6.1
react-native-elements: ^1.2.7 => 1.2.7
react-native-fs: ^2.16.6 => 2.16.6
react-native-gesture-handler: ^1.6.1 => 1.6.1
react-native-get-random-values: ^1.4.0 => 1.4.0
react-native-logs: ^2.2.1 => 2.2.1
react-native-paper: ^3.8.0 => 3.10.1
react-native-permissions: ^2.1.1 => 2.1.5
react-native-reanimated: ^1.8.0 => 1.9.0
react-native-safe-area-context: ^0.7.3 => 0.7.3
react-native-screens: ^2.4.0 => 2.9.0
react-native-sound-player: ^0.10.4 => 0.10.4
react-native-svg: ^12.1.0 => 12.1.0
react-native-svg-transformer: ^0.14.3 => 0.14.3
react-native-vector-icons: ^6.6.0 => 6.6.0
react-native-webview: =9.2.1 => 9.2.1
react-native-zip-archive: ^5.0.4 => 5.0.4
react-test-renderer: 16.11.0 => 16.11.0
recyclerlistview: ^3.0.0 => 3.0.0
npmGlobalPackages:
@aws-amplify/cli: 4.22.0
ios-deploy: 1.10.0
npm: 6.14.4
react-native-cli: 2.0.1
Smartphone (please complete the following information):
- Device: iPad
- OS: iOS 13
- React Native
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 1
- Comments: 17 (12 by maintainers)
Commits related to this issue
- reproduce: aws-amplify/amplify-js#6363 — committed to wei/aws-amplify-react by wei 4 years ago
@nubpro and @laclance Update: we’ve identified the issue and are working on a solution for this specifically when using Automerge as the conflict resolution strategy.
Note that this problem is exclusive to Automerge. Optimistic Concurrency abides by last-write-wins, so it will correctly keep the latter update.
Clients never update the version in DataStore. The consistency model is to have a central authority and therefore only AppSync updates the version. This is outlined in the AppSync documentation as well.
Up-leveling to the original question: If I make a local mutation, then change the data again before going online, the final state is what should be represented in the system. If that’s not happening then this is a bug and should be looked into. DataStore’s offline queue is a hashmap which operates in the following mechanism:
With this information lets go back to the original question:
Based on this description I suspect a bug exists in the React Native implementation for draining the hashmap after a successful service confirmation of a write. We should review this in bugbash.
I can reproduce in React too. Repo link &
git checkout 6363.Local data/mutations seems to be working fine, it’s the appsync graphql request that is returning the previous data when
_versionremains unchanged.For example I’m sending 5 saves A B C D E: https://github.com/wei/aws-amplify-react/blob/6363/src/App.js#L21-L27
2 Appsync graphql requests can be observed:
I tried to replay the second request as well sending the same
_versionand I can observe that_versionand_lastChangedAtare being updated butnameremain asTitle AWhen setting
_versionto the exact_versionreturned in the latest appsync graphql request, the request will work fine and return the new name.When setting
_versionto a number higher than_versionpreviously returned by appsync, an error is returned with messageClient version is greater than the corresponding server version. Therefore, we cannot just increment_versionon the client-side.The second request in my screenshots is updating
_versionand_lastChangedAtbut notnamewhich looks like it’s appsync’sAUTOMERGEat play. I’ll try with react native next.Update I can reproduce it in React Native as well. It is sending the exact same two graphql requests as seen in the screenshots above after network is restored. I’l do some more digging on the DataStore’s offline queue / hashmap undefobj mentioned in the comment below.
@danrivett I think you’re reading too much into the phrasing of my comment. The library should handle all of these implementations internally, not force the developer to think about them. @wei is one of the Amplify team fellows working with @iartemiev
Please don’t do this, let AppSync handle updates to the version. Modifying it could lead to data loss.
I’ve spoken with @iartemiev and I think there is an improvement to the hashmap implementation for slow networks that we can make by not allowing the write to the record when the item is in flight. He’s tracking the issue internally.
@undefobj Thanks for sharing your insights.
I performed some tests on react-native. Take my example of updating titles to
A B C D Ein sequence.There are two different scenarios:
Reachability - Notifying reachability change falsebefore sending all 5 saves. Everything is working as expected - Once network is back on and reachability becomes true, only one graphql mutation request is sent to appsync forE.Awhich will take a while. In the meantimeB C D Esaves are called and the outbox correctly removesB C Dand leaving onlyE. Once graphql mutation request forAis finished, the request forEis sent. Since the two requests have the same_version,AUTOMERGEwill ignore the second update (E) to “title” (a scala field). (See screenshots in my first reply)To summarize: This issue arises when a request is enroute and another mutation on the same model is triggered in the meantime (with the same
_version).@nubpro One solution I can think of is to use a custom LAMBA conflict resolver to basically ignore _version and always update the field, not ideal but doable.