On Fri, 2011-09-30 at 12:10 +0200, Patrick Ohly wrote: > This does not yet address the situation where the remote side also > wasn't changed. A redundant Replace command will be sent in that case. > Do you have a hint for me how this can be avoided? In the server case, > is it as simple as avoiding the SendItemAsServer(augmentedItemP) > call?
That really seems to do the trick. Does the attached patch look right? It works for me (TM). -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter.
>From 92ca341dfd67b3cbc4178b57b66a2f24439fa178 Mon Sep 17 00:00:00 2001 From: Patrick Ohly <[email protected]> Date: Fri, 30 Sep 2011 12:58:20 +0200 Subject: [PATCH] DB_Conflict (409): avoid sending unnecessary changes back to client In server mode, when the merging after a 409 from the DB determines that the client already has the latest data, sending it a Replace command can be avoided. This patch achieves that by introducing a new boolean remoteHasLatestData which is false by default and only set when it is certain that sending the update can be avoided. To minimize changes, the DB_DataReplaced code path is left unchanged. It could be changed to set the same boolean. Not sure whether that will also avoid unnecessary changes sent when a client detects a conflict. --- src/sysync/customimplds.cpp | 24 ++++++++++++++---------- src/sysync/customimplds.h | 2 +- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/sysync/customimplds.cpp b/src/sysync/customimplds.cpp index 338f4d9..d3c32c8 100755 --- a/src/sysync/customimplds.cpp +++ b/src/sysync/customimplds.cpp @@ -2720,7 +2720,7 @@ localstatus TCustomImplDS::implProcessMap(cAppCharP aRemoteID, cAppCharP aLocalI /// helper to merge database version of an item with the passed version of the same item; /// does age comparison by default, with "local side wins" as fallback -TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion) +TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion, bool &aChangedNewVersion) { aChangedDBVersion = false; @@ -2754,16 +2754,15 @@ TMultiFieldItem *TCustomImplDS::mergeWithDatabaseVersion(TSyncItem *aSyncItemP, "Incoming item is newer and wins" : "DB item is newer and wins")); - bool changedNewVersion; if (crstrategy==cr_client_wins) { - aSyncItemP->mergeWith(*dbVersionItemP, changedNewVersion, aChangedDBVersion, this); + aSyncItemP->mergeWith(*dbVersionItemP, aChangedNewVersion, aChangedDBVersion, this); } else { - dbVersionItemP->mergeWith(*aSyncItemP, aChangedDBVersion, changedNewVersion, this); + dbVersionItemP->mergeWith(*aSyncItemP, aChangedDBVersion, aChangedNewVersion, this); } PDEBUGPRINTFX(DBG_DATA,( "Merged incoming item (%s,relevant,%smodified) with version from database (%s,%s,%smodified)", crstrategy==cr_client_wins ? "winning" : "loosing", - changedNewVersion ? "" : "NOT ", + aChangedNewVersion ? "" : "NOT ", crstrategy==cr_server_wins ? "winning" : "loosing", aChangedDBVersion ? "to-be-replaced" : "to-be-left-unchanged", aChangedDBVersion ? "" : "NOT " @@ -2839,6 +2838,7 @@ bool TCustomImplDS::implProcessItem( } // - now perform op aStatusCommand.setStatusCode(510); // default DB error + bool remoteHasLatestData = false; switch (sop) { /// @todo sop_copy is now implemented by read/add sequence /// in localEngineDS, but will be moved here later possibly @@ -2863,8 +2863,8 @@ bool TCustomImplDS::implProcessItem( if (sta==DB_Conflict) { // DB has detected item conflicts with data already stored in the database and // request merging current data from the backend with new data before storing. - bool changedDBVersion; - augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion); + bool changedDBVersion, changedNewVersion; + augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion, changedNewVersion); if (augmentedItemP==NULL) sta = DB_Error; // no item found, DB error else { @@ -2886,6 +2886,10 @@ bool TCustomImplDS::implProcessItem( fLocalItemsUpdated++; sta = DB_DataMerged; } + // in the processing below avoid sending an unnecessare Replace + // if the data sent by the peer already was up-to-date + if (!changedNewVersion) + remoteHasLatestData = true; } } if (IS_SERVER) { @@ -2935,7 +2939,7 @@ bool TCustomImplDS::implProcessItem( } // if backend has not replaced, but merely merged data, we're done. Otherwise, client needs to be updated with // merged/augmented version of the data - if (sta!=DB_DataReplaced) { + if (!remoteHasLatestData && sta!=DB_DataReplaced) { // now create a replace command to update the item added from the client with the merge result // - this is like forcing a conflict, i.e. this loads the item by local/remoteid and adds it to // the to-be-sent list of the server. @@ -2989,8 +2993,8 @@ bool TCustomImplDS::implProcessItem( if (sta==DB_Conflict) { // DB has detected item conflicts with data already stored in the database and // request merging current data from the backend with new data before storing. - bool changedDBVersion; - augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion); + bool changedDBVersion, changedNewVersion; + augmentedItemP = mergeWithDatabaseVersion(myitemP, changedDBVersion, changedNewVersion); if (augmentedItemP==NULL) sta = DB_Error; // no item found, DB error else { diff --git a/src/sysync/customimplds.h b/src/sysync/customimplds.h index 939f1ad..f6ef8ee 100755 --- a/src/sysync/customimplds.h +++ b/src/sysync/customimplds.h @@ -773,7 +773,7 @@ protected: // as a item copy with only finalisation-required fields void queueForFinalisation(TMultiFieldItem *aItemP); /// helper to merge database version of an item with the passed version of the same item - TMultiFieldItem *mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion); + TMultiFieldItem *mergeWithDatabaseVersion(TSyncItem *aSyncItemP, bool &aChangedDBVersion, bool &aChangedNewVersion); public: // - get last to-remote sync time lineartime_t getPreviousToRemoteSyncCmpRef(void) { return fPreviousToRemoteSyncCmpRef; }; -- 1.7.2.5
_______________________________________________ os-libsynthesis mailing list [email protected] http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
