1321021 - Automatically organize Other Bookmarks when there are too many children

Status: NEW

(Firefox :: Sync, defect, P3)

Description

[Richard Newman [:rnewman]]

There's a hard limit to the size of a Sync record.

When a folder has more than about 16,000 children, the child GUID list alone exceeds that limit, and bookmark Sync will break.

Even before that point, each time you hit the bookmark star, Firefox will upload 256KB of data.

We should impose a limit — perhaps 5,000 records — and automatically create subfolders in Other Bookmarks when it has more children than the limit.

The Firefox UI can seamlessly "flatten" these folders if we wish, but a structure would exist in Places.

There are other approaches to solving this problem:

* Prompt the user!
* Have the bookmark star automatically file in dated folders!
* Give up on the existing bookmarks Sync format!

but I think this is a reasonable hack to get these extreme users unblocked with minimal work.

Thoughts?

Comment 1

[Richard Newman [:rnewman]]

It's worth noting that "break" here is both silent and expensive: bookmark syncing will immediately cease, for all folders, and each retry costs at least 256KB of upload bandwidth until we fix this bug or the user realizes there's a problem.

Comment 2

[Lina Butler [:lina]]

I agree; this would be simplest and least invasive to do in Places.

I was going to suggest extending the Sync record format to support chunking, maybe using suffixed record IDs like like "OCz96bUh4MVA/1", "OCz96bUh4MVA/2", and so on. But that would confuse older clients, and encode important information in a field that's meant to be opaque. We could go further and have Sync chunk children into virtual folders, but now it needs to maintain (and sync!) the mapping.

In the UI, "Other Bookmarks" is already a query. Could we change it to point to multiple folders? ("folder=UNFILED_BOOKMARKS_1&folder=UNFILED_BOOKMARKS_2&folder=UNFILED_BOOKMARKS_3").

Comment 3

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

I'm inclined to say that as long as users with less than x0000 bookmarks in their "other bookmarks" see no changes, the number of users actually impacted by this will be so small that doing something silent (eg, just automatically selecting/creating a subfolder to put it in) and not bothering with having the UI re-flatten them would be fine. Thom, it should be possible for our existing telemetry to tell us how many users hit this, right?

Comment 4

[Marco Bonardo [:mak]]

(In reply to Richard Newman [:rnewman] from comment #0)
> Even before that point, each time you hit the bookmark star, Firefox will
> upload 256KB of data.

Why is this?
Adding a bookmark that way doesn't even update all the other bookmarks position, since it's appended. it would merely bump its ancestors lastModified.
Is this an intrinsic limit of the sync bookmarks format?

Regardless, having so many bookmarks in a single folder is also a problem for our UI, so splitting into folders could help there. Maybe could even just be yearly folders. The best would likely be "auto-folders", based on the category, but then we'd need an online service able to put pages into categories.

Though, this workaround is a bit lacking:
1. users could have thousands of bookmarks in the menu, toolbar, any other subfolder. Are we only concerned about Starring?
2. users could unlike and misunderstand our foldering, and just move back all the bookmarks to a single folder. What will we do then? How do we prevent the user from adding the 5001st bookmark?
3. how complex would be to make Sync not upload the world for a single bookmark addition?

Comment 5

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

(In reply to Marco Bonardo [::mak] from comment #4)
> (In reply to Richard Newman [:rnewman] from comment #0)
> > Even before that point, each time you hit the bookmark star, Firefox will
> > upload 256KB of data.
> 
> Why is this?

The record for a folder stores a list of children GUIDs. A new bookmark will cause sync to write the child and parent.

> Though, this workaround is a bit lacking:
> 1. users could have thousands of bookmarks in the menu, toolbar, any other
> subfolder. Are we only concerned about Starring?

Nope - as comment 0 implies, 16k bookmarks in any folder is a problem. Unfiled is just the most common offender. Bug I don't think we'd be doing users with >16k bookmarks in menu or toolbar a disservice by limiting that number somehow.

> 2. users could unlike and misunderstand our foldering, and just move back
> all the bookmarks to a single folder. What will we do then? How do we
> prevent the user from adding the 5001st bookmark?

The places API could enforce this by ignoring/adjusting the specified parent.

> 3. how complex would be to make Sync not upload the world for a single
> bookmark addition?

very difficult :( Changing the server format to be something like relational really isn't feasible in any medium-term I can foresee.

That said though, a doubling (say) of the max record size probably is possible and would make a tiny minority or broken sync users even smaller. Although I suspect those users are already having a bad time in various ways and that they'd, in general, be helped by something like this in more ways than just sync.

Comment 6

[Richard Newman [:rnewman]]

Yes, increasing the max record size would help, if all of their uploading devices were running current Firefox, and at the cost of uploading, say, 500KB every time they hit the star. 

Many of these users seem to want to not manage their bookmarks at all — that is, they are "starrers", not "filers". I don't think those folks will notice at all. 

To be clear, I suggest this as an expedient hack: if you have Sync set up and you're trying to sync 15,000 Other Bookmarks, we will make it work by doing some organization on your behalf. No changes needed to the Sync protocol or servers, and minimal change to Places.

Comment 7

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

Yes, this is something we can compute via telemetry. It's a little tricky since we don't have longitudinal sync info, but still doable -- Let me know if I should.

What isn't in our data is how many users would be effected by re-foldering with 5000 per folder. -- That is, the number of users with >= 5k entries in "Other Bookmarks".

Comment 8

[Richard Newman [:rnewman]]

It's probably fair to assume that a large proportion of users with more than 20,000 bookmarks have at least 15,000 of them in Unsorted Bookmarks. We do have some "mega organizers" in our user base, but I expect they're dwarfed by the "Ctrl-D-ers".

You can also ask bobm to see what the record-too-large error rate is for bookmarks. If these users use Sync, they'll fail repeatedly, and it should be quite obvious. (Of course, we've probably driven many of them away.)

Comment 9

[Lina Butler [:lina]]

If we buffer incoming and outgoing records, chunking could happen transparently. The buffer could wait to apply incoming bookmarks until it has the complete folder, and split large folders into multiple records before uploading them in the same batch. This assumes that all clients support buffering, and will know how to handle these partial records...we'd need a collection version bump.

Comment 10

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

As mentioned above, having the new storage server allow double the BSO size seems low-effort and high-impact. This solution still makes sense for the very-long tail, but might allow us to put it off even further still...