Automatically organize Other Bookmarks when there are too many children

NEW
Unassigned

Status

()

defect
P3
normal
3 years ago
2 months ago

People

(Reporter: rnewman, Unassigned)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [data-integrity])

Reporter

Description

3 years ago
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?
Reporter

Comment 1

3 years ago
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.
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").
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?
Flags: needinfo?(tchiovoloni)
(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?
(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.
Reporter

Comment 6

3 years ago
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.
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".
Flags: needinfo?(tchiovoloni)
Reporter

Comment 8

3 years ago
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.)
Priority: -- → P3
Whiteboard: [data-integrity]
Reporter

Updated

2 years ago
See Also: → 1300451
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.
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox

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...

You need to log in before you can comment on or make changes to this bug.