drewmccormack / ensembles

A synchronization framework for Core Data.
MIT License
1.63k stars 132 forks source link

Multipeer does not sync both ways... #233

Closed pronebird closed 8 years ago

pronebird commented 8 years ago

I've been playing around multipeer sync and it seems that if I create a CD record on remote device, local device is being notified about it via status message.

However when status message arrives with the list of files on remote device, local device compares two sets of files, but it only checks for files missing on remote device, regardless whether remote device has more files / more changes.

Therefore the change is being unnoticed because remote device has more files than local device and NSSet simply returns empty intersection against local files.

This logic is located in handleFileRetrievalRequestFromPeerWithID of CDEMultipeerCloudFileSystem.m.

It would be awesome to actually download new stuff from remote device and send CDEMultipeerCloudFileSystemDidImportFilesNotification to perform the merge once data is available.

drewmccormack commented 8 years ago

It is by design that each peer treats all other peers as if they are servers. But the way it does that is to simply make a local cache of files, and that is actually what it syncs against.

So you have to call retrieveFilesFromPeersWithIDs: yourself in order to get the new files. That puts the files in the local cache, and then you can call merge to bring the changes into the Core Data store. This approach gives you more control over when you do the data transfers, and when you integrate the results into Core Data.

pronebird commented 8 years ago

Peer message decoding happens in CDEMultipeerCloudFileSystem(receiveData:fromPeerWithID:). Therefore I can't know the remote files that other peer has without doing the same decoding twice in IDMMultipeerManager. That's why additional notification to tell the manager that new files are available on other peer is a good way to organize real time synchronization. iCloud file system does that and polls files in iCloud container, then pings back with notification that new files are available. I think multipeer should do too.

drewmccormack commented 8 years ago

But you have total control over when the files are retrieved, calling retrieveFilesFromPeersWithIDs:. It is true you don't know if there is new data, but if there is not, calling merge is very cheap. So you can just always call merge after calling retrieveFilesFromPeersWithIDs: (or use a different schedule).

pronebird commented 8 years ago

Sorry if I am being silly here. I monitor CDEMonitoredManagedObjectContextDidSaveNotification and call merge on each save.

The following snippet does not work for me:

[self.ensemble mergeWithCompletion:^(NSError *error) {
    [self.multipeerManager syncFilesWithAllPeers];
}];

Even if I put another one merge after syncFilesWithAllPeers, it does not push files to other peer, nothing happens. Can that be that the timing is not right?

drewmccormack commented 8 years ago

I think you would need two merges to be fully synced. So you need to do the first merge to export the new save data to the local file cache. Then you call syncFilesWithAllPeers to put those files on other devices, and to also retrieve any new files from those other devices. Lastly, you would need another merge to import those changes from other devices.

I think this code would work too:

[self.multipeerManager syncFilesWithAllPeers];
[self.ensemble mergeWithCompletion:^(NSError *error) {
    [self.multipeerManager syncFilesWithAllPeers];
}];
pronebird commented 8 years ago

Oh yes, you were right. The problem was that I forgot to pass multipeerManager at some point into my operation. 😫

drewmccormack commented 8 years ago

Note also that your code (IDMMultipeerManager) is the code that sends and receives data, so you can always put extra stuff in there. The CDEMultipeerCloudFileSystem uses delegate methods to ask your app code to do the sending/receiving.

Glad it is working anyway.

pronebird commented 8 years ago

Ugh, I don't know why it worked. Eventually it stopped working again. And nothing is merged until I manually sync on other device. So frustrating.. I tried to run sync like 5 times in a row on save on the device that produces changes and other peer does not see these changes at all. I run sync on other peer, and it works. and I logged everything in console and it sees the peer after save and attempts to push the changes there:

-[MLEnsembleMultipeerManager syncFilesWithAllPeers] line 94: syncFilesWithAllPeers: (
    "<MCPeerID: 0x7f88d0ca3aa0 DisplayName = Andrey's iPhone>"
)
drewmccormack commented 8 years ago

I tried to run sync like 5 times in a row on save on the device that produces changes and other peer does not see these changes at all.

In order for the other device to see those changes:

  1. The saving device has to call mergeWithCompletion:.
  2. The other device has to call syncFilesWithAllPeers.
  3. The other device has to call mergeWithCompletion:

Are you saying those things are happening, but the data is not appearing? Or are you saying it is not working because there is nothing to trigger 2 and 3?

One way to do the above is just to run a syncFiles and merge every 30 secs or so. If you need more spontaneous updates, you will probably need to send a message yourself between devices, to trigger a merge on the other device.

If you think it would be better to have a delegate method in Ensembles, perhaps you can try adding one and make a pull request.

pronebird commented 8 years ago

@drewmccormack thanks for trying to help me.

Indeed the problem that other peers don't call sync because there is nothing to trigger that. I understand that timer approach is easy to implement, but I think it's nicer when changes arrive as they happen. iCloud and CloudKit both have ways to achieve that.

I think it would be awesome to add a new type of CDEMultipeerMessageType and invite other devices to synchronize with the peer that produced changes.

drewmccormack commented 8 years ago

Sounds like a good request. But note also that you could already setup your own message using the same sessions you control, and call that whenever a monitored store is saved into. That would send the message to other peers, and they could trigger a merge.