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

Issue 893969 link

Starred by 1 user
Status: WontFix
Owner:
mamir@chromium.org
Closed: Nov 19
Cc:
mastiz@chromium.org
ios-bugs@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: 2019-03-31
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug



Sign in to add a comment

[Sync] Updates have duplicate server ids and duplicate client tag hashes.

Project Member Reported by mamir@chromium.org, Oct 10 
We got recent reports that when processing updates to the processors in the USS architecture, there are updates that have duplicate server id 
and duplicate client tag hashes.

We should add deduplication logic for both the server id, and the client tag hashes.

However, server id deduplication should be sufficient. We added UMA metrics to detect client tag hashes duplicates before and after the server id deduplication.

If the UMA metric shows that only server deduplication is sufficient, we should remove the logic for the client tag hashes deduplication.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 10 

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

commit ea43384275f737c991b4cfaa51fee4614faac159
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Oct 10 18:19:49 2018

[Sync] Deduping updates with the same server id

Some recent reports indicate that the Sync server might be sending
updates with duplicate sync ids. This is considered "invalid data",
and is so far not handled well by the client.

This CL adds the logic to remove updates with duplicate sync ids
(and keep only the latest)

In addition, it adds metrics to evaluate how common this is.

Existing logic for deduplication based on the client tag hashes already
existed. UMA metrics are also added to if deduplication based on
sync ids is sufficient or both deduplications are needed.

Bug:  893969 , 893998

Change-Id: Ia7106623a47a3c6ddd12b452fbc4bdfa75c0fe44
Reviewed-on: https://chromium-review.googlesource.com/c/1270944
Reviewed-by: Robert Kaplow (sloooow) <rkaplow@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598408}
[modify] https://crrev.com/ea43384275f737c991b4cfaa51fee4614faac159/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/ea43384275f737c991b4cfaa51fee4614faac159/components/sync/engine_impl/model_type_worker.h
[modify] https://crrev.com/ea43384275f737c991b4cfaa51fee4614faac159/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/ea43384275f737c991b4cfaa51fee4614faac159/tools/metrics/histograms/histograms.xml

Comment 2 by mastiz@chromium.org, Nov 16

Labels: -Pri-3 sync-fixit-2018q4 Sync-Triaged Pri-2

Comment 3 by mamir@chromium.org, Nov 19

Status: WontFix (was: Assigned)
Looking at the uma metrics Sync.DuplicateClientTagHashWithDifferentServerIdsInApplyPendingUpdates.*
Most of the data types look OK and don't have any duplicate client tag hashes after server id duplication.

Preferences and TypesURLs however, had very small number of duplicate (less than 1 per million).
This could be very corner case that birthday changes in the middle of a get updates cycles for example. This could be also some noise in UMA reporting.

The decision is
1- Leave both deduplication logics in place to avoid crashes. 
2- Since the number is negligible, no more investigation is required!

Comment 4 by tschumann@chromium.org, Nov 19 

Also, to close the loop; the we configured our datastore, it's possible for ids to be included in more than one response.
►

Sign in to add a comment