diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-05-26 00:06:47 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-05-26 00:06:47 +0000 |
commit | ce0f41696951562391ad3d1bf180c2ae08db66fb (patch) | |
tree | a502e5d771252e26df5f51477eb477696a2dc7f7 /sync/engine/commit.cc | |
parent | 085e92da549d6e20df4e023687aad2c23e05aea2 (diff) | |
download | chromium_src-ce0f41696951562391ad3d1bf180c2ae08db66fb.zip chromium_src-ce0f41696951562391ad3d1bf180c2ae08db66fb.tar.gz chromium_src-ce0f41696951562391ad3d1bf180c2ae08db66fb.tar.bz2 |
sync: Loop committing items without downloading updates
Since its inception sync has required all commits to be preceded by a
GetUpdates request. This was done to try to ensure we detect and resolve
conflicts on the client, rather than having two conflicting commits
collide at the server. It could never work perfectly, but it was likely
to work in most cases and the server would bounce the commit if it
didn't.
Now we have a new server that doesn't always give us the latest version
of a node when we ask for it, especially when the request was not
solicited by the server (ie. not triggered by notification). The update
before commit is now much less likely to detect conflicts. Even worse,
the useless update requests can be surprisingly expensive in certain
scenarios.
This change allows us to avoid fetching updates between 'batches' of
commits. This should improve performance in the case where we have lots
of items to commit (ie. first time sync) and reduce load on the server.
This CL has some far-reaching effects. This is in part because I decided
to refactor the commit code, but major changes would have been required
with or without the refactor. Highlights include:
- The commit-related SyncerCommands are now paramaterized with local
variables, allowing us to remove many members from the SyncSession
classes.
- Several syncer states have been collapsed into one COMMIT state
which redirects to the new BuildAndPostCommits() function.
- The PostCommitMessageCommand had been reduced to one line, which
has now been inlined into the BuildAndPostCommits() function.
- The unsynced_handles counter has been removed for now. Following this
change, it would have always been zero unless an error was encountered
during the commit. The functions that reference it expect different
behaviour and would have been broken by this change.
- The code to put extensions activity data back into the
ExtensionsActivityMonitor in case of failure has been moved around to
avoid double-counting if we have to post many commit messages.
- A CONFLICT response from the server is now treated as a transient
error. If we had continued to treat it as a success we might risk
going into an infinite loop. See comment in code for details.
- The "purposeless anachronism" conflicting_new_folder_ids_ has been
removed.
Review URL: https://chromiumcodereview.appspot.com/10210009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@139159 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/engine/commit.cc')
-rw-r--r-- | sync/engine/commit.cc | 100 |
1 files changed, 100 insertions, 0 deletions
diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc new file mode 100644 index 0000000..ae6b8ae --- /dev/null +++ b/sync/engine/commit.cc @@ -0,0 +1,100 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sync/engine/commit.h" + +#include "base/debug/trace_event.h" +#include "sync/engine/build_commit_command.h" +#include "sync/engine/get_commit_ids_command.h" +#include "sync/engine/process_commit_response_command.h" +#include "sync/engine/syncer_proto_util.h" +#include "sync/sessions/sync_session.h" + +using syncable::SYNCER; +using syncable::WriteTransaction; + +namespace browser_sync { + +using sessions::SyncSession; +using sessions::StatusController; + +// Helper function that finds sync items that are ready to be committed to the +// server and serializes them into a commit message protobuf. It will return +// false iff there are no entries ready to be committed at this time. +// +// The OrderedCommitSet parameter is an output parameter which will contain +// the set of all items which are to be committed. The number of items in +// the set shall not exceed the maximum batch size. (The default batch size +// is currently 25, though it can be overwritten by the server.) +// +// The ClientToServerMessage parameter is an output parameter which will contain +// the commit message which should be sent to the server. It is valid iff the +// return value of this function is true. +bool PrepareCommitMessage(sessions::SyncSession* session, + sessions::OrderedCommitSet* commit_set, + ClientToServerMessage* commit_message) { + TRACE_EVENT0("sync", "PrepareCommitMessage"); + + commit_set->Clear(); + commit_message->Clear(); + + WriteTransaction trans(FROM_HERE, SYNCER, session->context()->directory()); + sessions::ScopedSetSessionWriteTransaction set_trans(session, &trans); + + // Fetch the items to commit. + const size_t batch_size = session->context()->max_commit_batch_size(); + GetCommitIdsCommand get_commit_ids_command(batch_size, commit_set); + get_commit_ids_command.Execute(session); + + DVLOG(1) << "Commit message will contain " << commit_set->Size() << " items."; + if (commit_set->Empty()) + return false; + + // Serialize the message. + BuildCommitCommand build_commit_command(*commit_set, commit_message); + build_commit_command.Execute(session); + + return true; +} + +SyncerError BuildAndPostCommits(Syncer* syncer, + sessions::SyncSession* session) { + StatusController* status_controller = session->mutable_status_controller(); + + sessions::OrderedCommitSet commit_set(session->routing_info()); + ClientToServerMessage commit_message; + while (PrepareCommitMessage(session, &commit_set, &commit_message) + && !syncer->ExitRequested()) { + ClientToServerResponse commit_response; + + DVLOG(1) << "Sending commit message."; + TRACE_EVENT_BEGIN0("sync", "PostCommit"); + status_controller->set_last_post_commit_result( + SyncerProtoUtil::PostClientToServerMessage(commit_message, + &commit_response, + session)); + TRACE_EVENT_END0("sync", "PostCommit"); + + // ProcessCommitResponse includes some code that cleans up after a failure + // to post a commit message, so we must run it regardless of whether or not + // the commit succeeds. + + TRACE_EVENT_BEGIN0("sync", "ProcessCommitResponse"); + ProcessCommitResponseCommand process_response_command( + commit_set, commit_message, commit_response); + status_controller->set_last_process_commit_response_result( + process_response_command.Execute(session)); + TRACE_EVENT_END0("sync", "ProcessCommitResponse"); + + // Exit early if either the commit or the response processing failed. + if (status_controller->last_post_commit_result() != SYNCER_OK) + return status_controller->last_post_commit_result(); + if (status_controller->last_process_commit_response_result() != SYNCER_OK) + return status_controller->last_process_commit_response_result(); + } + + return SYNCER_OK; +} + +} // namespace browser_sync |