Mimetis / Dotmim.Sync

A brand new database synchronization framework, multi platform, multi databases, developed on top of .Net Standard 2.0. https://dotmimsync.readthedocs.io/
MIT License
899 stars 194 forks source link

Strange behavior on synchronization #288

Closed NunzioCar closed 4 years ago

NunzioCar commented 4 years ago

We are using your syncronization components and we tried out, in some cases, a strange behavior! We need help to understand if the behavior is correct or if there is a bug.

We have a client that works offline and that periodically synchronize with a server (with the purge of the metadata on client side (CleanMetadatas=true)). The user can use both to read and update entities. The client uses SQLite for persistence and the server uses SQL Server. Both use EF Core for the data access layer and we prepare the DB using the EF migrations.

The tested scenarios are:

  1. we delete a record X on the client side, we change the same record on the server side and then we execute the synchronization from the client:

    we expected a RemoteExistsLocalIsDelete conflict, but we receive RemotedIsDeletedLocalIsDeleted and

    • if we choice, on the server side, in the OnApplyChangesFailed code, the ServerWins conflict resolution, the synchronization completes successfully.
    • if we choice the ClientWins, the synchronization completes without errors but without changes applied: the server has the record but the client not.
  2. we delete a record X on the server side, we change the same record on the client side and then we execute the synchronization from the client:

    we expected RemoteIsDeletedLocalExists conflict but we receive RemoteExistsLocalExists and

    • if we choice the ServerWins, the synchronization completes without errors but without changes applied: the client has the record but the server not.
  3. what’s meaning of RemoteIsDeletedLocalIsDeleted conflict? Why does it throw this conflict if client and server deletes the same record?

  4. we add a new record on the server side, update it on the server side and then we execute the synchronization from the client:

    • if we choice, on server side, in the OnApplyChangesFailed code, the ServerWins or ClientWins conflict resolution, the synchronization completes successfully.
    • if we choice the ConflictResolution.Rollback, the synchronization fails. Why is there a conflict? what's the meaning of RemoteExistsLocalNotExists?
  5. we add a new record on the server side, delete it on the server side and then we execute the synchronization from the client:

    • if we choice the ConflictResolution.Rollback the synchronization fails but we don't receive the conflict, the RemoteIsDeletedLocalNotExists conflict. What's the meaning of RemoteIsDeletedLocalNotExists?
  6. is there a way to manage the synchronization of a record with relationships (with foreign key)?

    we add a child record, on the client side, to an existing parent record, delete the parent record on the server side then we execute the synchronization on the client:

    • regardless we choice the ClientWins or ServerWins, the synchronization completes without errors, the parent record is deleted but the child not. So, we have an orphan child record!
  7. we delete a record X on the client side, after we update the same record on the server side and add a child record Y. Why in this case the conflict type is RemoteIsDeletedLocalIsDeleted?

    • if we choice the ClientWins, the records X and Y are not deleted.
  8. we update a record X on the client side and add a child Y, after we delete, on the server side, the record X. Then we execute the synchronization from the client:

    • if we choice ServerWins or ClientWins, the record X is deleted from both sides, instead the record Y is persisted on client and server like an orphan child. There was no conflict generated.
  9. we add, on the client side, a child record Y to an existing record X, after we add, on the server side, a record Z to the record X, in the end we delete the entity X and Z on server. Then we execute the synchronization from the client:

    • if we choice ServerWins or ClientWins, there is a resolved conflict, but the event OnApplyChangesFailed is never triggered. For this reason, the conflict type is unknown. Like result the orphan record Y was not cancelled and was persisted even on server.
  10. we add, on the client side, a child record Y, after we delete the entity X on the client side, on server we add a child record Z to X. Why in this case the conflict type is RemoteIsDeletedLocalIsDeleted?

    • if we choice the ClientWins, no changes was applied on the client and the server side and after synchronization.

To give to user a greater control on syncronization we need some other features:

Mimetis commented 4 years ago

Hello @NunzioCar Thank you so much for this well detailed issue !

I'm actually working on your issue. Thank to all your detailed descriptions, I was able to find some bugs in the conflict resolution.

I will make a more detailed answer later this week, once I've done and resolved everything. I just need a liitle bit more time to be sure everything is running correctly and to be sure I can answer each of your question points.

seb

Mimetis commented 4 years ago

Hi @NunzioCar

I've worked a lot this week on the conflict resolution mechanism.
First of all, Thank You So Much for all the detailed scenarios you provided.
It helped me A LOT.

I have created dedicated tests to cover the most scenario as possible here : TcpConflictsTests.cs

Will try to answer each scenario. You will see that some behaviors have been corrected, and some other are intended. Let's go !

Conflict type

First of all, regarding the ConflictType enumeration, we have a wording like Remote and Local and not Server and Client. In a sense that would have been easier to have Server / Client, but depending the situation, it would not work.

The resolution takes place in both side: Firstly on the server then on the client side.
So far, the RemoteXXX and LocalXXX have different meaning:

1) If you are on the server side (remoteOrchestrator.OnApplyChangesFailed), then:

2) Then if you are handling the error from the client side (localOrchestrator.OnApplyChangesFailed), then:

Here is a piece of code that illustrates this behavior.
We are resolving a conflict occuring when a row has been deleted on the client, and updated on the server

var agent = new SyncAgent(client.Provider, Server.Provider, options, new SyncSetup(Tables));

var localOrchestrator = agent.LocalOrchestrator;
var remoteOrchestrator = agent.RemoteOrchestrator;

// From the client : Remote is server and Local is client
localOrchestrator.OnApplyChangesFailed(acf =>
{
    Assert.Equal(ConflictType.RemoteExistsLocalIsDeleted, acf.Conflict.Type);
});

// From Server : Remote is client and Local is server
remoteOrchestrator.OnApplyChangesFailed(acf =>
{
    Assert.Equal(ConflictType.RemoteIsDeletedLocalExists, acf.Conflict.Type);
});

As you can see, on the server we have a conflict type of RemoteIsDeletedLocalExists and from the client perspective, the opposite conflict type RemoteExistsLocalIsDeleted.

Your scenarios

We delete a record X on the client side, we change the same record on the server side.

Here you should have (now) a RemoteIsDeletedLocalExists conflict type, from the server side. You have the tests here:

(DC for Deleted from Client) (US for Updated from Server)

https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L797

https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L861

We delete a record X on the server side, we change the same record on the client side

You should have now a RemoteIsDeletedLocalExists conflict type from the server side. You have the tests here:

https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1126

https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1190

What’s meaning of RemoteIsDeletedLocalIsDeleted conflict ?

Indeed, you have a row deleted on both side. But DMS need to track who should have done the delete in the first place.
In that particular case, the only thing done here is the update of the metadata store (the _tracking row is updated in consequence)

Tests here: https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1291

https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1349

We add a new record on the server side, update it on the server side

Strange, it's a regular update here, so you should not have any conflict raised here ...

We add a new record on the server side, delete it on the server side

Tests here

https://github.com/Mimetis/Dotmim.Sync/blob/9f06d0d1e6a2eee50f5a614addcad869ff124bb4/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1576

https://github.com/Mimetis/Dotmim.Sync/blob/9f06d0d1e6a2eee50f5a614addcad869ff124bb4/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1624

Is there a way to manage the synchronization of a record with relationships (with foreign key)?

We add a child record, on the client side, to an existing parent record, delete the parent record on the server side then we execute the synchronization on the client.

Unfortunately, there is no solution to resolve this problem.

DMS makes a synchronization of tables. You can have some problems, regarding orphans rows and so on.

The only solution you have is to fake update the parent record on the client side, to be sure you will have a conflict raised and then make the appropriate solution.

We delete a record X on the client side, after we update the same record on the server side and add a child record Y. Why in this case the conflict type is RemoteIsDeletedLocalIsDeleted?

You should now have a RemoteIsDeletedLocalExists conflict type raised.

We update a record X on the client side and add a child Y, after we delete, on the server side, the record X.

You should now have a RemoteExistsLocalIsDeleted conflict type raised.

Tests here: https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1126

https://github.com/Mimetis/Dotmim.Sync/blob/6ef08f8d3ce517854a30614ab5627d3d158ac2bc/Tests/Dotmim.Sync.Tests/TcpConflictsTests.cs#L1190

We add, on the client side, a child record Y to an existing record X, after we add, on the server side, a record Z to the record X, in the end we delete the entity X and Z on serve

DMS relies on foreign keys constraints. Your record Y should raise an foreign key constraint error in that case.

Features

Conflict resolution on the client side

It's possible, but you need to make two synchronization in a row to be able to allow the user to choose which one is the good one.
If the Two Syncs In A Row is acceptable for you, it's possible.
I need to make a sample, let me know if you are interested in

Is there a way to get all the conflicts to show to the user?

Yes, he can see the conflicts, but only once they are resolved from the server side (then he can override the result and re sync, it's actually the scenario I describe in your previous question)

Is there a way to know if there is something to synchronize?

Yep:

var serverProvider = new SqlSyncProvider(DBHelper.GetDatabaseConnectionString(serverDbName));
var clientProvider = new SqlSyncProvider(DBHelper.GetDatabaseConnectionString(clientDbName));
var setup = new SyncSetup(new string[] { "Address", "Customer", "CustomerAddress", "SalesOrderHeader", "SalesOrderDetail" });

var options = new SyncOptions();
// Creating an agent that will handle all the process
var agent = new SyncAgent(clientProvider, serverProvider, options, setup);

var changes = await agent.LocalOrchestrator.GetChangesAsync()
NunzioCar commented 4 years ago

Hi Sébastien, thanks, great job.

About the sense of Remote and Local, ok it's clear.

About the record with relationships I think that we could simply ignore the orphan child.

About the We add a new record on the server side, update it on the server side scenario we will check it.

About the Two sync trick, after a brainstorming, we think that will keep the sync simple: we introduce a soft delete to not lost any record (better for orphan child too) and handling conflicts manually only in the server side, in HTTP mode. In our application we can suppose that the changes are made from the same user from multi client so

About the way to know if there is something to synchronize, ok we can use LocalOrchestrator.GetChangesAsync() in the client side; this to get the client changes, right? But if we would to get the remote changes too, we should execute the following code too:

// Get client scope var clientScope = await localOrchestrator.GetClientScopeAsync(); // Get changes to be populated to the server var changes = await remoteOrchestrator.GetChangesAsync(clientScope);

This code works in HTTP mode? Or do we have to add a custom controller action to get the remoteOrchestrator.GetChangesAsync(clientScope) result?

Thanks a lot and sorry I wasted your time.

Nunzio

Mimetis commented 4 years ago

Hey @NunzioCar

You don't waste my time ! Actually, you helped me A LOT

LocalOrchestrator.GetChangesAsync() will get all the changes from the client to be uploaded to the server RemoteOrchestrator.GetChangesAsync(clientScope) will get all the changes from the server to be downloaded and applied to the client (excepting any conflict that may happen !)

For the Get Changes from remote side, you have a sample here RemoteOrchestrator.GetChanges

This code works locally, but if you want to get the changes from the client side, in HTTP mode, using a WebClientOrchestrator instance, it's not implemented (yet):

public class WebClientOrchestrator : RemoteOrchestrator
{
        public override Task<(long RemoteClientTimestamp, BatchInfo ServerBatchInfo, DatabaseChangesSelected ServerChangesSelected)> GetChangesAsync(ScopeInfo clientScope, ServerScopeInfo serverScope = null, CancellationToken cancellationToken = default, IProgress<ProgressArgs> progress = null)
            => throw new NotImplementedException();
}

I will implement the method later on, maybe for the release of the next version 0.5.6.
Until then, as you say, you have to implement a custom controller. If you can wait a couple of days, it should be available on the master branch, once I've implemented it.

Be careful, this method get the changes from the server, excepting all the conflicts that may arise...

NunzioCar commented 4 years ago

Hi Sébastien, yes, we can wait your implementation of WebClientOrchestrator.GetChangesAsync(). We need only a changes count estimation so your implementation is enough.

Thanks again

Mimetis commented 4 years ago

Hey the GetEstimatedChangesCount() seems to be a good idea to implement !

Mimetis commented 4 years ago

I've implemented the Two methods in all Orchestrators (LocalOrchestrator, RemoteOrchestrator and WebClientOrchestrator):

You have some useful tests here:

Still need to fix some edges issues, but it should fine at the end of the day.
Then I will publish a new beta version on nuget.

Let me know if it's useful for you, then.

gentledepp commented 4 years ago

@Mimetis - very, very helpful indeed! For example: When switching to a different remote, our users have to log out. In order for no data to be "lost" as it was not yet uploaded to server A, we can now determine, if there are any pending changes to be send and trigger a sync in that case.

NunzioCar commented 4 years ago

GetEstimatedChangesCount is what we need: we will use this function to inform the user that there are many changes on client and\or on server and so that is required a sync.

Thanks

Mimetis commented 4 years ago

Tests are green :)

You can get the beta version 0.5.6-beta-0430 with these 2 new methods, from nuget, otherwise you can use the master branch source code directly.

NunzioCar commented 4 years ago

Hi Sébastien, the Dotmim.Sync.Sqlite assembly is not updated, right? When do you think to update this assembly?

Thanks

Mimetis commented 4 years ago

Sorry, my pipeline for SQLite was disabled. It's resolved, you should be able to get the last beta version now

NunzioCar commented 4 years ago

Can you publish Dotmim.Sync.Core 0.5.6-beta-0432? Thanks

Mimetis commented 4 years ago

I've just made a full rebuild / republish All components are available as version 0.5.6-beta-0433

NunzioCar commented 4 years ago

Hi Sebastien, we are testing some sync scenarios and all seems working properly.

I have a question: when a record is changed on server side and on client side we need to choice the last updated record. To do that we need a last-update-date field so is there, in the conflict info, a last update date for the remote and the local row? Or do I have to add a custom field in our tables?

Thanks

Mimetis commented 4 years ago

Well, you can rely on the last_change_datetime from the table_tracking table. Be careful, this datetime is set there for information only. It's not used by the DMS framework to compare rows, internally.

The DateTime format is perfect as a human readable thing but for row comparison, the DMS fx relies on the TimeStamp column that is different and using mostly a bigint value.

For you scenario I guess this Datetime value should be enough.

NunzioCar commented 4 years ago

Ok, clear. So I have to read this datetime directly from database and it's not available in the SyncConflict and SyncRow, right? Thanks

Mimetis commented 4 years ago

Yes, it's not available in the SyncConflict, yet. Maybe in a future release.

I will update this post then, but don't expect it until ... I don't know when :D

NunzioCar commented 4 years ago

Ok Sebastien, thanks for all.

Great work