Skip to content

Conversation

jsflax
Copy link
Contributor

@jsflax jsflax commented Oct 28, 2018

The main addition of this ticket is the added method resumeSyncForDocument. A major secondary change is that the language for "frozen/thawed" has been switched to "paused/resumed". This is the documentation for this method:

/**
   * A document that is paused no longer has remote updates applied to it.
   * Any local updates to this document cause it to be resumed. An example of pausing a document
   * is when a conflict is being resolved for that document and the handler throws an exception.
   *
   * @param documentId the id of the document to resume syncing
   * @return true if successfully resumed, false if the document
   *         could not be found or there was an error resuming
*/
boolean resumeSyncForDocument(@Nonnull final BsonValue documentId);

This method is proxied upwards from DataSynchronizer => CoreSync => Sync.

The tests assert that, in the event of an irrecoverable error, a document is paused, and that document can be resumed via this method. The method should return the appropriate boolean value if the resume was successful. Operations should then be able to be sync'd on said document.

* @return true if successfully resumed, false if the document
* could not be found or there was an error resuming
*/
boolean resumeSyncForDocument(@NonNull final BsonValue documentId);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good. In addition to this, we need a way to get the Set of all documents that are frozen

}

config.setPaused(false);
return !config.isPaused();
Copy link
Contributor

Choose a reason for hiding this comment

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

won't this always be true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought there's a small chance of a race here– document is resumed and in that instant an error occurs again. Returning !isPaused seemed like a sure way to return the state of the doc. I could leave a TODO for your threading ticket. Or I could be being overly careful here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like it's fine to return true. The same thing can happen even right after it returns to you.

Copy link
Contributor

@edaniels edaniels left a comment

Choose a reason for hiding this comment

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

Looking good, we just need the ability to list paused docs as well. My bad not specifying this in the ticket.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 64.239% when pulling 6b77141 on jsflax:STITCH-2133 into 1679a78 on mongodb:master.

@coveralls
Copy link

coveralls commented Oct 28, 2018

Coverage Status

Coverage increased (+0.2%) to 64.296% when pulling 341d305 on jsflax:STITCH-2133 into 1679a78 on mongodb:master.

@jsflax jsflax merged commit acc5c32 into mongodb:master Oct 28, 2018
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.

3 participants