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

[WIP] Remote sync revamp #319

Conversation

wooj2
Copy link
Contributor

@wooj2 wooj2 commented Feb 16, 2020

Don't review this. This is actually four separate commits I will be splitting up in the very near future.

Uploading this to a branch in the event my computer gets hit by a bus.

Notable updates:

  • remote sync engine was moved to a serial statemachine
  • handle a sendCompletion (failure) in order to kick the state machine to an errored/clean up restart
  • Added cancel interface to cancel event reconciliation queue, model reconciliation queue, listeners, mappers, etc.
  • Added sending connection information from API in order to decide when subscribeInitialization succeeds (e.g., if we can't make a subscription connection, then we shouldn't be doing an initialSync)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@available(iOS 13.0, *)
func onReceiveCompletion(receiveCompletion: Subscribers.Completion<DataStoreError>) {
let semaphore = DispatchSemaphore(value: 1)
semaphore.wait()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guessing this gets called multiple times and you only want it called once? can you removeDuplicates() on the sink that calls this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout -- and actually this may not be needed at all

What I was actually trying to protect against is not multiple calls on a single subscription - but I think there was a bug in my clean up logic, where i was only pausing the queue (rather than destroying it), which lead to multiple onReceiveCompletions.

@@ -7,7 +7,7 @@

import Amplify
import Combine

//How much of this we will use vs trash, i have no idea
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, whooops that was removed . in a subsequent pr :P

@wooj2
Copy link
Contributor Author

wooj2 commented Feb 16, 2020

Closing this pr.. as i have split this PR into 4 distinct PRs:
#321 - migrating to a state machine
#322 -added retry logic to sync engine, without canceling downstream async operations
#323 - added in the cancelling interface
#324 - cancelling the downstream operations & wait for subscription to init.

@wooj2 wooj2 closed this Feb 16, 2020
@wooj2 wooj2 deleted the master-RemoteSyncEngineToSerialStateMachine-HandleRestart-cancelInterface-fixConnectionPropagation branch February 20, 2020 01:13
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.

2 participants