Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(179)

Side by Side Diff: sync/engine/process_commit_response_command.cc

Issue 11411041: sync: Retry soon when server returns conflict (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 8 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "sync/engine/process_commit_response_command.h" 5 #include "sync/engine/process_commit_response_command.h"
6 6
7 #include <cstddef> 7 #include <cstddef>
8 #include <set> 8 #include <set>
9 #include <string> 9 #include <string>
10 #include <vector> 10 #include <vector>
(...skipping 153 matching lines...) Expand 10 before | Expand all | Expand 10 after
164 return SERVER_RETURN_TRANSIENT_ERROR; 164 return SERVER_RETURN_TRANSIENT_ERROR;
165 } else if (conflicting_commits > 0) { 165 } else if (conflicting_commits > 0) {
166 // This means that the server already has an item with this version, but 166 // This means that the server already has an item with this version, but
167 // we haven't seen that update yet. 167 // we haven't seen that update yet.
168 // 168 //
169 // A well-behaved client should respond to this by proceeding to the 169 // A well-behaved client should respond to this by proceeding to the
170 // download updates phase, fetching the conflicting items, then attempting 170 // download updates phase, fetching the conflicting items, then attempting
171 // to resolve the conflict. That's not what this client does. 171 // to resolve the conflict. That's not what this client does.
172 // 172 //
173 // We don't currently have any code to support that exceptional control 173 // We don't currently have any code to support that exceptional control
174 // flow. We don't intend to add any because this response code will be 174 // flow. Instead, we abort the current sync cycle and start a new one. The
175 // deprecated soon. Instead, we handle this in the same way that we handle 175 // end result is the same.
176 // transient errors. We abort the current sync cycle, wait a little while, 176 return SERVER_RETURN_CONFLICT;
177 // then try again. The retry sync cycle will attempt to download updates
178 // which should be sufficient to trigger client-side conflict resolution.
179 //
180 // Not treating this as an error would be dangerous. There's a risk that
181 // the commit loop would loop indefinitely. The loop won't exit until the
182 // number of unsynced items hits zero or an error is detected. If we're
183 // constantly receiving conflict responses and we don't treat them as
184 // errors, there would be no reason to leave that loop.
185 //
186 // TODO: Remove this option when the CONFLICT return value is fully
187 // deprecated.
188 return SERVER_RETURN_TRANSIENT_ERROR;
189 } else { 177 } else {
190 LOG(FATAL) << "Inconsistent counts when processing commit response"; 178 LOG(FATAL) << "Inconsistent counts when processing commit response";
191 return SYNCER_OK; 179 return SYNCER_OK;
192 } 180 }
193 } 181 }
194 182
195 void LogServerError(const sync_pb::CommitResponse_EntryResponse& res) { 183 void LogServerError(const sync_pb::CommitResponse_EntryResponse& res) {
196 if (res.has_error_message()) 184 if (res.has_error_message())
197 LOG(WARNING) << " " << res.error_message(); 185 LOG(WARNING) << " " << res.error_message();
198 else 186 else
(...skipping 276 matching lines...) Expand 10 before | Expand all | Expand 10 after
475 // been recursively deleted. 463 // been recursively deleted.
476 // TODO(nick): Here, commit_message.deleted() would be more correct than 464 // TODO(nick): Here, commit_message.deleted() would be more correct than
477 // local_entry->Get(IS_DEL). For example, an item could be renamed, and then 465 // local_entry->Get(IS_DEL). For example, an item could be renamed, and then
478 // deleted during the commit of the rename. Unit test & fix. 466 // deleted during the commit of the rename. Unit test & fix.
479 if (local_entry->Get(IS_DIR) && local_entry->Get(IS_DEL)) { 467 if (local_entry->Get(IS_DIR) && local_entry->Get(IS_DEL)) {
480 deleted_folders->insert(local_entry->Get(ID)); 468 deleted_folders->insert(local_entry->Get(ID));
481 } 469 }
482 } 470 }
483 471
484 } // namespace syncer 472 } // namespace syncer
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698