Commit 9b4773ae authored by zea@chromium.org's avatar zea@chromium.org

[Sync] Remove logging/checks for case where server and local entries mismatch.

We will now reliably hit this code due to the server changing the MTIME of any
committed item. As a result of this, and because the check and log is expensive,
it's better to just remove them altogether.

BUG=103838
TEST=none


Review URL: http://codereview.chromium.org/8526018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@109761 0039d316-1c4b-4281-b951-d872f2087c98
parent 6dbc070c
...@@ -148,16 +148,6 @@ ServerUpdateProcessingResult ProcessUpdatesCommand::ProcessUpdate( ...@@ -148,16 +148,6 @@ ServerUpdateProcessingResult ProcessUpdatesCommand::ProcessUpdate(
SyncerUtil::UpdateServerFieldsFromUpdate(&target_entry, update, name); SyncerUtil::UpdateServerFieldsFromUpdate(&target_entry, update, name);
if (target_entry.Get(syncable::SERVER_VERSION) ==
target_entry.Get(syncable::BASE_VERSION) &&
!target_entry.Get(syncable::IS_UNSYNCED) &&
!target_entry.Get(syncable::IS_UNAPPLIED_UPDATE)) {
// If these don't match, it means that we have a different view of the
// truth from other clients. That's a sync bug, though we may be able
// to recover the next time this item commits.
LOG_IF(ERROR, !SyncerUtil::ServerAndLocalEntriesMatch(&target_entry))
<< target_entry;
}
return SUCCESS_PROCESSED; return SUCCESS_PROCESSED;
} }
......
...@@ -473,68 +473,6 @@ void SyncerUtil::CreateNewEntry(syncable::WriteTransaction *trans, ...@@ -473,68 +473,6 @@ void SyncerUtil::CreateNewEntry(syncable::WriteTransaction *trans,
} }
} }
// static
bool SyncerUtil::ServerAndLocalOrdersMatch(syncable::Entry* entry) {
// Find the closest up-to-date local sibling by walking the linked list.
syncable::Id local_up_to_date_predecessor = entry->Get(PREV_ID);
while (!local_up_to_date_predecessor.IsRoot()) {
Entry local_prev(entry->trans(), GET_BY_ID, local_up_to_date_predecessor);
if (!local_prev.good() || local_prev.Get(IS_DEL))
return false;
if (!local_prev.Get(IS_UNAPPLIED_UPDATE) && !local_prev.Get(IS_UNSYNCED))
break;
local_up_to_date_predecessor = local_prev.Get(PREV_ID);
}
// Now find the closest up-to-date sibling in the server order.
syncable::Id server_up_to_date_predecessor =
entry->ComputePrevIdFromServerPosition(entry->Get(SERVER_PARENT_ID));
return server_up_to_date_predecessor == local_up_to_date_predecessor;
}
// static
bool SyncerUtil::ServerAndLocalEntriesMatch(syncable::Entry* entry) {
if (entry->Get(CTIME) != entry->Get(SERVER_CTIME)) {
LOG(WARNING) << "Client and server time mismatch";
return false;
}
if (entry->Get(IS_DEL) && entry->Get(SERVER_IS_DEL))
return true;
// Name should exactly match here.
if (!(entry->Get(NON_UNIQUE_NAME) == entry->Get(SERVER_NON_UNIQUE_NAME))) {
LOG(WARNING) << "Unsanitized name mismatch";
return false;
}
if (entry->Get(PARENT_ID) != entry->Get(SERVER_PARENT_ID) ||
entry->Get(IS_DIR) != entry->Get(SERVER_IS_DIR) ||
entry->Get(IS_DEL) != entry->Get(SERVER_IS_DEL)) {
LOG(WARNING) << "Metabit mismatch";
return false;
}
if (!ServerAndLocalOrdersMatch(entry)) {
LOG(WARNING) << "Server/local ordering mismatch";
return false;
}
// TODO(ncarter): This is unfortunately heavyweight. Can we do better?
if (entry->Get(SPECIFICS).SerializeAsString() !=
entry->Get(SERVER_SPECIFICS).SerializeAsString()) {
LOG(WARNING) << "Specifics mismatch";
return false;
}
if (entry->Get(IS_DIR))
return true;
// For historical reasons, a folder's MTIME changes when its contents change.
// TODO(ncarter): Remove the special casing of MTIME.
if (entry->Get(MTIME) != entry->Get(SERVER_MTIME)) {
LOG(WARNING) << "Time mismatch";
return false;
}
return true;
}
// static // static
void SyncerUtil::SplitServerInformationIntoNewEntry( void SyncerUtil::SplitServerInformationIntoNewEntry(
syncable::WriteTransaction* trans, syncable::WriteTransaction* trans,
...@@ -772,17 +710,6 @@ VerifyResult SyncerUtil::VerifyUpdateConsistency( ...@@ -772,17 +710,6 @@ VerifyResult SyncerUtil::VerifyUpdateConsistency(
return VERIFY_FAIL; return VERIFY_FAIL;
} }
if (target->Get(ID) == update.id()) { if (target->Get(ID) == update.id()) {
// Checks that are only valid if we're not changing the ID.
if (target->Get(BASE_VERSION) == update.version() &&
!target->Get(IS_UNSYNCED) &&
!SyncerProtoUtil::Compare(*target, update)) {
// TODO(sync): This constraint needs to be relaxed. For now it's OK to
// fail the verification and deal with it when we ApplyUpdates.
LOG(ERROR) << "Server update doesn't match local data with same "
"version. A bug should be filed. Entry: " << *target <<
"Update: " << SyncerProtoUtil::SyncEntityDebugString(update);
return VERIFY_FAIL;
}
if (target->Get(SERVER_VERSION) > update.version()) { if (target->Get(SERVER_VERSION) > update.version()) {
LOG(WARNING) << "We've already seen a more recent version."; LOG(WARNING) << "We've already seen a more recent version.";
LOG(WARNING) << " Entry: " << *target; LOG(WARNING) << " Entry: " << *target;
......
...@@ -68,8 +68,6 @@ class SyncerUtil { ...@@ -68,8 +68,6 @@ class SyncerUtil {
static void CreateNewEntry(syncable::WriteTransaction *trans, static void CreateNewEntry(syncable::WriteTransaction *trans,
const syncable::Id& id); const syncable::Id& id);
static bool ServerAndLocalEntriesMatch(syncable::Entry* entry);
static void SplitServerInformationIntoNewEntry( static void SplitServerInformationIntoNewEntry(
syncable::WriteTransaction* trans, syncable::WriteTransaction* trans,
syncable::MutableEntry* entry); syncable::MutableEntry* entry);
...@@ -130,11 +128,6 @@ class SyncerUtil { ...@@ -130,11 +128,6 @@ class SyncerUtil {
const syncable::ScopedDirLookup &dir, const syncable::ScopedDirLookup &dir,
std::set<syncable::Id>* deleted_folders); std::set<syncable::Id>* deleted_folders);
// Examine the up-to-date predecessors of this item according to the server
// position, and then again according to the local position. Return true
// if they match. For an up-to-date item, this should be the case.
static bool ServerAndLocalOrdersMatch(syncable::Entry* entry);
private: private:
DISALLOW_IMPLICIT_CONSTRUCTORS(SyncerUtil); DISALLOW_IMPLICIT_CONSTRUCTORS(SyncerUtil);
}; };
......
...@@ -16,6 +16,7 @@ import pickle ...@@ -16,6 +16,7 @@ import pickle
import random import random
import sys import sys
import threading import threading
import time
import urlparse import urlparse
import app_notification_specifics_pb2 import app_notification_specifics_pb2
...@@ -82,6 +83,9 @@ SYNC_TYPE_TO_EXTENSION = { ...@@ -82,6 +83,9 @@ SYNC_TYPE_TO_EXTENSION = {
# The parent ID used to indicate a top-level node. # The parent ID used to indicate a top-level node.
ROOT_ID = '0' ROOT_ID = '0'
# Unix time epoch in struct_time format. The tuple corresponds to UTC Wednesday
# Jan 1 1970, 00:00:00, non-dst.
UNIX_TIME_EPOCH = (1970, 1, 1, 0, 0, 0, 3, 1, 0)
class Error(Exception): class Error(Exception):
"""Error class for this module.""" """Error class for this module."""
...@@ -462,6 +466,9 @@ class SyncDataModel(object): ...@@ -462,6 +466,9 @@ class SyncDataModel(object):
entry.originator_client_item_id = base_entry.originator_client_item_id entry.originator_client_item_id = base_entry.originator_client_item_id
self._entries[entry.id_string] = copy.deepcopy(entry) self._entries[entry.id_string] = copy.deepcopy(entry)
# Store the current time since the Unix epoch in milliseconds.
self._entries[entry.id_string].mtime = (int((time.mktime(time.gmtime()) -
time.mktime(UNIX_TIME_EPOCH))*1000))
def _ServerTagToId(self, tag): def _ServerTagToId(self, tag):
"""Determine the server ID from a server-unique tag. """Determine the server ID from a server-unique tag.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment