1341923 - Record telemetry on number of tombstones in our collections.

Status: RESOLVED INVALID

(Firefox :: Sync, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

In bug 1332290 we are considering raising the TTL for tombstones to prevent scenarios causing old deleted items to be resurrected. We need to give some indication to operations on how this will impact storage, so knowing an average number of tombstones stored for each collection would be helpful.

We can't measure this at the back-end as the deleted flag is in the encrypted payload. On a "first sync" for a device we do read all records from the server, but that's not likely to be helpful (as that is likely to happen when a sync account is young, and hasn't yet accumulated many tombstones.) However, when we perform validation we do read every record on the server, so this might be a good place to record that.

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1341923 - Record a telemetry histogram for number of tombstones seen during sync validation

Review commit: https://reviewboard.mozilla.org/r/117612/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/117612/

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

My understanding is that we don't just want this for bookmarks, but since we only really run validation on bookmarks, it's unclear if that's really the right time to do it (it might be).

It's also really tempting to do this as an extra property on the validation data (e.g. serverTombstones), but this breaks the invariant that all those should be 0 for a successful validation, which I think we still want to keep since it keeps a lot of stuff simple.

Also this (as written) probably needs data-review after you give the r+, but I'll wait until that happens to request it.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8844056 [details]
Bug 1341923 - Record a telemetry histogram for number of tombstones seen during sync validation

https://reviewboard.mozilla.org/r/117612/#review119490

::: services/sync/modules/bookmark_validator.js:121
(Diff revision 1)
>   * - clientMissing: Array of ids on the server missing from the client
>   * - serverMissing: Array of ids on the client missing from the server
>   * - serverDeleted: Array of ids on the client that the server had marked as deleted.
>   * - serverUnexpected: Array of ids that appear on the server but shouldn't
>   *   because the client attempts to never upload them.
> + * - serverTombstones: Array of tombstones that appear on the server.

I don't see this variable being maintained and it looks the same as serverDeleted?

::: services/sync/modules/collection_validator.js:194
(Diff revision 1)
>            problems.differences.push({ id, differences });
>          }
>        }
>      }
> +
> +    let histogram = Services.telemetry.getHistogramById("WEAVE_TOMBSTONE_COUNT");

I don't think this will tell us what we want to know. IIUC, what we will end up with is, say, 5 different recordings of the same telemetry histogram, with each value recording a different number (ie, each for a different collection). If it so happens that a validation runs multiple times in a session, that session will record those same values multiple times. I don't see how that would allow us to deduce the average number of tombstones a single account has (which is what we are trying to answer, so ops can work out the cost of extending the TTL for tombstones).

Please let me know if I'm wrong though! Data is hard :(

Comment 4

[Lina Butler [:lina]]

We recently migrated everyone, so there are no tombstones to record. Putting this back into our triage queue for later. We'd still like to know this, but it's not actionable now.