1332094 - Incremental sync follow up: transactionally merge buffers

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Android Sync, defect, P5)

Description

[:Grisha Kruglov]

Merging of a buffer should be done transactionally, whenever possible. One way to facilitate this is via the buffering session middleware: add signaling to let the wrapped session know that we're about to start merging, and that we're done.

Comment 1

[:Grisha Kruglov]

Some notes on implementing this.

The idea here is that we know we are about to start merging in a buffer full of records, and we'd like to do so transactionally.

To expand on what's described in Comment 0, perhaps one crude approach is to create/end transactions via BrowserProvider's call interface from our wrapped RepositorySessions (which are told to do so by BufferingMiddlewareRepositorySession [0]). We must really pay attention here to ensure transactions aren't left open in case of problems, are committed properly, etc.

Perhaps a better approach is to accept that this kind of buffering is here to stay, and to introduce a new RepositorySession `storeRecords` interface. Instead of looping at [0], we can hand over records to the inner repository and assume it'll "do the right thing". Local sessions tend to perform record reconciliation in their `store` methods, and build up queues of records to flush. If we want to avoid refactoring much of this flow in this bug, one incremental improvement would be to ensure that these queues are large enough (or likely unbounded) so that they're able to hold all of the changes that might be in a buffer, and to flush them in a way that's possibly very similar to how history data accessor transactionally flushes queues of records via BrowserProvider's call interface (see [1] and [2]). Special attention must be paid to bookmarks, as it has separate queues for insertions and deletions of folders and non-folders.

A tractable approach might be to land these changes separately for different repositories, or perhaps tackle changes for bookmarks separately from the rest of collections, as they're inherently a bit more complicated.

Whatever approach to this is attempted, it must at least consider how user's interactions with existing data during the buffer merge process data might affect things.

[0] https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/src/main/java/org/mozilla/gecko/sync/middleware/BufferingMiddlewareRepositorySession.java?q=path%3ABufferingMiddlewareRepositorySession.java&redirect_type=single#131-139
[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java?q=path%3AHistoryDataAccessor&redirect_type=single#133-139
[2] https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java#2245

Comment 2

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 3

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.