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

SyncManager: report engines on/off status as soon as possible #2161

Closed
grigoryk opened this issue Nov 13, 2019 · 6 comments
Closed

SyncManager: report engines on/off status as soon as possible #2161

grigoryk opened this issue Nov 13, 2019 · 6 comments

Comments

@grigoryk
Copy link
Contributor

grigoryk commented Nov 13, 2019

Current behaviour is that status of engines - if they're enabled or disabled - is reported once sync is finished, even though it could be reported as soon as the meta/global collection is processed. This delay leads to undesired behaviour: pressing "Sync Now", then checking/unchecking a sync engine is likely to flip that checkbox back to what it was. If client requested a follow-up sync for sync engine state (as it should), in a few seconds that checkbox may flip back again.

In clients that don't have complete control over sync scheduling (Fenix, FxR using Android's WorkManager), sync requests that happen very close to each other in time may be dropped, or delayed, which leads to non-deterministic behaviour that's made worse by this time gap.

A callback-driven API would help here. This callback doesn't need to be long-lived - my expectation is that it'll get triggered once early on during a sync, after which we can drop it.

┆Issue is synchronized with this Jira Task
┆Epic: Important backlog

@rfk
Copy link
Contributor

rfk commented Nov 14, 2019

A callback-driven API would help here.

I'm not sure I follow what you mean here - do you mean a kind of "we found the engine choices, here they are" notification callback that gets send form the syncmanager to the application at that early stage of the sync after metaglobal is processed?

@jdragojevic
Copy link
Contributor

jdragojevic commented Nov 14, 2019

FxR team is hoping to have the fix for this in their release scheduled for 12/12, which has a QA pass scheduled for 11/27.

@rfk
Copy link
Contributor

rfk commented Nov 14, 2019

Given that callback-driven APIs have historically been kind of hard for us, I think we need to do a bit of brainstorming on the correct approach ASAP if we want to have something testable by late November.

I wonder if it would be cleaner to break the sync into two distinct phases, once for "do the meta stuff to get ready to sync" and one for "now sync all the engines using that state" that the caller can execute independently. But I'm not close enough to the code here to know if that would be workable. @grigoryk perhaps you could sketch a strawfox API for the change you'd like to see?

@thomcc
Copy link
Contributor

thomcc commented Nov 19, 2019

We could probably do a callback API if it were very limited and we were deliberate about what could be done from the callback (a lot of things will either deadlock or crash).

What would be the desired action to take inside the callback? Just update the UI?

@keianhzo
Copy link

@grigoryk any idea about when is this going to be triaged?

@mhammond
Copy link
Member

mhammond commented Sep 6, 2023

I don't think this has enough detail to make it useful to keep around.

@mhammond mhammond closed this as completed Sep 6, 2023
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

No branches or pull requests

6 participants