New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 656072 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Sync client should to exponential backoff when receive partial failure

Project Member Reported by gangwu@chromium.org, Oct 14 2016

Issue description

When sync client received PARTIAL_FAILURE, client should perform exponential backoff instead of throttled backoff.

 

Comment 1 by zea@chromium.org, Oct 14 2016

Do we support per-datatype throttling using the THROTTLED response? We should make sure we support both per-datatype throttling and exponential backoff.

Comment 2 by gangwu@chromium.org, Oct 14 2016

We only support per-datatype THROTTLED, but for exponential backoff, we do not support per-datatype currently I think.

Comment 3 by gangwu@chromium.org, Oct 18 2016

Labels: Hotlist-Polish
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dcb17ac181e0c58d3bee7fece41e712df434f904

commit dcb17ac181e0c58d3bee7fece41e712df434f904
Author: gangwu <gangwu@chromium.org>
Date: Tue Nov 15 20:55:58 2016

[Sync] Sync client should to exponential backoff when receive partial failure.

BUG= 656072 

Before this CL, Sync will throttle the data types in
error_data_type_ids, which is wrong, Sync should exponential backoff
those data type.

This CL also fix several bugs.
1. When Sync client receive error THROTTLED, and error_data_type_ids is set,
Sync client will throttle those datatypes, and also throttle whole client.
This is wrong, the whole client should not get throttled in this case.
To fix this, when this case happened, SyncerProtoUtil::PostClientToServerMessage
will throttled the data types, and return PARTIAL_FAILURE instead of THROTTLED.

2. |scheduled_nudge_time_| in SyncSchedulerImpl is not update correctly,
so Sync client will sync less frequently than expected.
To fix this, I delete |scheduled_nudge_time_| and get the time directly from
|global_wakeup_timer_| (previously pending_wakeup_timer_).

3. When Sync client received PARTIAL_FAILURE, not only the data type in
error_data_type_ids got throttled, the whole client got exponential backoff.
To fix this, add an early return in SyncSchedulerImpl::HandleFailure when
receive PARTIAL_FAILURE.

Review-Url: https://codereview.chromium.org/2475043002
Cr-Commit-Position: refs/heads/master@{#432255}

[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine/sync_status.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/all_status.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/all_status.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/data_type_tracker.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/data_type_tracker.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/nudge_tracker.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/nudge_tracker.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/nudge_tracker_unittest.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/sync_cycle.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/test_util.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/cycle/test_util.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/get_updates_delegate.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/get_updates_processor.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/sync_engine_event_listener.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/sync_manager_impl.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/sync_scheduler_impl.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/sync_scheduler_impl.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/sync_scheduler_impl_unittest.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/syncer_proto_util.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/engine_impl/syncer_unittest.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/test/engine/fake_sync_scheduler.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/test/engine/fake_sync_scheduler.h
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/test/engine/mock_connection_manager.cc
[modify] https://crrev.com/dcb17ac181e0c58d3bee7fece41e712df434f904/components/sync/test/engine/mock_connection_manager.h

Status: Fixed (was: Assigned)

Sign in to add a comment