Improve concurrency when synchronizing

[gentledepp]

• dotmim-sync does use a single transaction to read all the changes of one scope.
• Say your scope is spanning dozens of tables, then there are quite a bunch of read-locks on them until all changes have been read.

-> Now if another user syncs then applying his changes may fail due to a transaction deadlock.

We solved this by creating a separate version of dotmim.sync that uses one transaction per table when fetching changes.
This does have the downside, that you may see records that you were not supposed to see.
eg. Order, OrderDetail tables.
OrderDetail has foreign key to Order. Therefore you have to synchronzie these tables in the order

• Order
• OrderDetail
  Now using our approach you could end up with an OrderDetail record sent to your client, that points to an Order that was not synchronized to your device yet.

The best option, in this case (and in my humble opinion), would be to take the "currentTimestamp" (taken the first instant the sync request receives the server) and, instead of using transactions for selecting changes, create a separate transaction for each table and
select all changes where timestamp > lastSyncedTimestamp && timestamp < currentTimestamp

That would give you the best of both worlds:

• no single transaction
• no non-consistent change sets

@Mimetis what do you think?

[Mimetis]

Transaction

What's the benefit to have one transaction per table (for reading) ?
I mean, if we reduce the transaction to a single table (for reading), there is ... basically no need to create a transaction ?

That's being said, should we consider to be able to remove transactions when reading changes (at least through an option or a mechanism like interceptors ?)

Regarding the issue #695, should we also consider to be able to remove transactions for writing as well ?

For the read lock, you have the option to set an IsolationLevel on your provider.
Is it enough ?

Dotmim.Sync/Projects/Dotmim.Sync.Core/CoreProvider.cs

Line 88 in f992c9e

public virtual IsolationLevel IsolationLevel { get; set; } = IsolationLevel.ReadCommitted;

I don't know if removing transactions will have more benefits than drawbacks, but adding an option to remove transaction during a particular step should not be too complicated, I guess.
But let's discuss it to be sure I understand everything :)

Timestamp

For the timestamp, in a nutshell, this value does not have any relationship with the in progress transaction.

• lastSyncedTimestamp is stored (copying the currentTimestamp value once the sync is done)
• currentTimestamp is generated just before making the sync process (just before getting changes) on each side
  • Doing this prevents to have "ghosts" records that could be inserted / updated during the selecting changes step.
  • If we have a record inserted / updated during the getting changes step, well this record will be synced on the next sync.

Dotmim.Sync/Projects/Dotmim.Sync.Core/Orchestrators/LocalOrchestrator.cs

Line 112 in f992c9e

clientTimestamp = await this.InternalGetLocalTimestampAsync(this.GetContext(), runner.Connection, runner.Transaction, cancellationToken, progress).ConfigureAwait(false);

Dotmim.Sync/Projects/Dotmim.Sync.Core/Orchestrators/RemoteOrchestrator.cs

Line 355 in f992c9e

var remoteClientTimestamp = await this.InternalGetLocalTimestampAsync(ctx, runner.Connection, runner.Transaction, cancellationToken, progress);

There is no relationships between transactions and timestamp, because (especially on SQL Server) we are not using the min_active_rowversion version (based on transaction) because of the effect you are describing

Dotmim.Sync/Projects/Dotmim.Sync.SqlServer/Builders/SqlScopeBuilder.cs

Line 223 in f992c9e

public override DbCommand GetLocalTimestampCommand(DbConnection connection, DbTransaction transaction)

[gentledepp]

nice! You do not need a transaction then because you already limit the record set so you do not get changes that were committed before the client started synchronizing 👍
This is the only way to keep the sync set consistent.
Because even when having a single read-committed transaction spanning all select-changes calls, it could happen that

1. you get changes from the Orders table
2. the other transaction commits a new order o and some order details od1 and od2
3. you now get the (read-committed) changes from the OrderDetails table
   -> you'd now include od1 and od2 in your changeset which would then fail to be inserted on the client, because o is not included in the changeset and the order details refer to it.

Baseline: The single transaction does not really do anything well. It raises the possibility of sql-deadlocks though (which we had to fight a lot in the old DMS FX

[Mimetis]

let me rephrase your scenario, I think we don't get the OrderDetails :)

1. I get the currentTimestamp (after applying changes, and before getting any changes from any table)
2. I get changes from Orders table (from lastTimeStamp to currentTimestamp)
3. Another transaction commits a new order in Orders oand two OrderDetails od1 & od2
   • o gets a tracking timestamp value of currentTimestamp +1
   • od1 gets a tracking timestamp value of currentTimestamp +2
   • od3 gets a tracking timestamp value of currentTimestamp +3
4. I get changes from OrderDetails (from lastTimeStamp to currentTimestamp)
   • I don't get neither od1, neither od2
5. After getting all changes lastTimestamp becomes currentTimestamp
6. On the next sync, I will get o, od1 and od2

Make sense ?

[gentledepp]

yes - this is perfect indeed.
This way, you can also safely get rid of the "single transaction spanning all sp_select calls" as it brings no benefits but has the downside of deadlocks (if other clients try to write to a table that has a table-lock because the changeset is huge - eg.)

[Mimetis]

I'm on it ;)

[gentledepp]

take your time. This is only a discussion that I merely started to not forget this topic ;-)