Experience with CKSyncEngine in the Forked library

[lukaskubanek]

I can't comment on specifics, but can comment on CKSyncEngine. I used it for the Forked project, and have had so much trouble I am considering returning to the standard queue based CK sync API.

In particular, CKSyncEngine seems to be susceptible to race conditions, which can be deadly in sync systems. If you don't know the state of a record you are working with, or it could change as you work with it, you are in real trouble. In my experience, that seems to be the case with CKSyncEngine. There don't seem to be any guarantees.

I've had much better results with the original CloudKit queue based sync API. That is very consistent and reliable. I've been using it in the Ensembles framework for around 13 years, and it is quite solid. You can be much more confident about the state of your records and objects because you control the operations, and can serialize them where necessary.

It may be possible to use asynchronous streams or queues to make CKSyncEngine more predictable, I'm not sure. In any case, just using it naively with the delegate is fraught with problems. And often those problems aren't immediately obvious. It is good that you at least have a reproducible test failure. That is 90% of the battle won.

This is a continuation of the discussion originally started by @drewmccormack in #354 (comment).

[lukaskubanek]

Thanks @drewmccormack for sharing your insights. They felt relevant for the wider Point-Free community, which is why I’ve reposted your comment here.

I don’t yet have production experience with CKSyncEngine, as I’m still in the experimentation phase for my project. I had high hopes when looking into SQLiteData, but the lack of customizable conflict resolution is currently a blocker for me. I’ve been exploring what it would take to contribute that feature, though it’s turning out to be a much larger task than expected…

Looks like you have real-world experience with CKSyncEngine, so I wonder how do the race conditions you mentioned actually manifest. In particular, what do you mean by knowing the state of a record and then having it change underneath you? Could you elaborate on that a bit?

[drewmccormack]

The types of issues that can arise are that if you get edits out of order, you could lose data. So it is pretty serious.

What I suspect, but haven't got proof for (I should try to make some), is that CKSyncEngine will not always give you an error when you are committing a record that is not predicated on the latest version from the server. This is essential, because it forms the basis of the optimistic locking which guarantees data consistency.

You need to know that when you commit changes to a record, it will fail to upload should there be a more recent record on the server. It seems like there may be some race which can arise where it is possible to do this without error.

I really should see if I can come up with a simple test to prove this problem and submit it to apple.

[lukaskubanek]

Well, that’s worrying!

I’ve experienced bugs with the CKSyncEngine/CloudKit myself. For example, I ran into cases where saves ere treated as atomic even though neither the CKSyncEngine nor the underlying CKModifyRecordsOperation were configured that way. When hitting a regular error like .serverRecordChanged for a single record, all other records in the batch would also fail with .batchRequestFailed, which was unexpected. It seemed to be related to the use of encrypted fields and Advanced Data Protection, but we never fully figured that out. Instead, a workaround was introduced to the library that simply re-enqueues the failed records. See #315.

That said, what you’re saying sounds really serious. The whole premise of a CloudKit-based sync system is that it reports an error when encountering is a newer version of a record on the server during save. I haven’t personally heard of issues with CKSyncEngine behaving like this, but a good place to ask might be the #cloudkit channel in the iOS Folks Slack.

One thought, though… Aren’t you referring to a situation where you have pending changes scheduled for upload but CKSyncEngine decides to fetch server changes first, effectively resulting in a conflict-on-fetch scenario, which CloudKit isn’t capable of detecting?

[drewmccormack]

The way I usually do it, and I think is quite standard with any sync system, is to fetch the latest record, update the record, and upload the record. If it fails because the record on the server changed in the meantime, you do it all over again. So I always work with the most recent record, applying the local changes on top of those, and attempting the upload.

If it does decide to fetch new changes while I'm in the middle of this, I would hope it would give me an error when I try to commit my new changes. But maybe it is what you are saying: it doesn't consider that a conflict or something.

Maybe you are supposed to do your own optimistic locking, by copying the token when you start the op, and check the token just before committing. That would still be pretty silly, I think, because again, it would be wide open to races. The only code that can properly detect concurrent updates is the code that uploads, namely CKSyncEngine itself.

[lukaskubanek]

The way I usually do it, and I think is quite standard with any sync system, is to fetch the latest record, update the record, and upload the record. If it fails because the record on the server changed in the meantime, you do it all over again.

The general flow you’re describing matches my mental model.

If it does decide to fetch new changes while I'm in the middle of this, I would hope it would give me an error when I try to commit my new changes.

The question is what exactly does “in the middle” mean. I think the critical spot is when the CKSyncEngine asks for the next batch of records. If you use an older ancestor record than the recently synced one, I’d expect you hitting the .serverRecordChanged error due to the out-of-date change tag. When the fetched record was correctly saved already and picked up in nextRecordZoneChangeBatch(…) as the baseline, then it should go through on the server.

What I meant by not considering it a conflict is when the record is fetched and used to overwrite local pending changes without properly executing the merge logic. One needs to detect that there are local pending changes. But my impression is that you’re doing that already with your fancy model.

Maybe you are supposed to do your own optimistic locking, by copying the token when you start the op, and check the token just before committing.

Not sure what you mean by the “operation” and “committing.” With the sync engine you signal a pending change, which gets eventually picked up. Do you mean observing the “will send” event, storing the change tags, and checking them again when preparing the batch?

The only code that can properly detect concurrent updates is the code that uploads, namely CKSyncEngine itself.

That makes sense, but again, I haven’t yet run across issues with this basic lifecycle. You’ve previously mentioned that it doesn’t report the .serverRecordChanged error when it should. Can you elaborate on the scenario? Were you able to reproduce it?