692372 - Process no more than 1 megabyte and no more than 100 WBOs per POST batch

Status: VERIFIED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[:Atoll]

Please adjust sync-server to process no more than 1 megabyte of wbo submissions per batch (based on wbo data length), and no more than 100 wbo submissions per batch (based on total count of submissions).  These should be configurable values.

Doing this will involve returning intelligent guid responses from the server, as per below.  We cannot simply alter the Firefox client as there are 4+ sync clients including Firefox and there is no limit on either byte count or item count specified in the API.

http://docs.services.mozilla.com/storage/apis-1.1.html

POST https://server/pathname/version/username/storage/collection

Takes an array of WBOs in the request body and iterates over them, effectively doing a series of atomic PUTs with the same timestamp.

Returns a hash of successful and unsuccessful saves, including guidance as to possible errors:

{"modified": 1233702554.25,
 "success": ["{GXS58IDC}12", "{GXS58IDC}13", "{GXS58IDC}15",
             "{GXS58IDC}16", "{GXS58IDC}18", "{GXS58IDC}19"],
 "failed": {"{GXS58IDC}11": ["invalid parentid"],
            "{GXS58IDC}14": ["invalid parentid"],
            "{GXS58IDC}17": ["invalid parentid"],
            "{GXS58IDC}20":["invalid parentid"]}}

Comment 1

[:Atoll]

:rnewman, there is no API spec compliant reply that can be produced by the Zeus for POST requests.  If your resp.obj null errors are always re: POST, it would be critically important to know the byte size of those POSTs for further diagnostics.

Comment 2

[Toby Elliott [:telliott]]

We have a 256k restriction per wbo, but will allow 100 of them in a batch, which is too high. Need to adjust the splicing process to also consider payload size in the calculation.

Comment 3

[Richard Newman [:rnewman]]

(In reply to Richard Soderberg [:atoll] from comment #1)
> :rnewman, there is no API spec compliant reply that can be produced by the
> Zeus for POST requests.  If your resp.obj null errors are always re: POST,
> it would be critically important to know the byte size of those POSTs for
> further diagnostics.

Byte size in my case has always been small: something like 25KB for a single tab record. I'm certainly nowhere near any of the limits mentioned in this bug.

Comment 4

[Toby Elliott [:telliott]]

It occurs to me that this isn't the problem we think it is (it's a problem, but that's different).

The only wbos that the client sends that are anywhere near 256K are tabs. However, a) we don't write those to the db and b) we never send more than one from a client. 

Everything else should be well under 10k on average, no? Which suggests that these megapayloads are either coming to us in error, or from a different client.

Can we log incoming post body sizes? Not sure how to do that in nginx.

Comment 5

[Philipp von Weitershausen [:philikon]]

(In reply to Toby Elliott [:telliott] from comment #4)
> It occurs to me that this isn't the problem we think it is (it's a problem,
> but that's different).
> 
> The only wbos that the client sends that are anywhere near 256K are tabs.
> However, a) we don't write those to the db and b) we never send more than
> one from a client. 
> 
> Everything else should be well under 10k on average, no? Which suggests that
> these megapayloads are either coming to us in error, or from a different
> client.

Looking at the info/quota and info/collection_counts numbers from my own account, I find that the typical size of a bookmark, history or forms record (as reported by info/quota) is about 0.5 KB.

Prefs are about 12.5 KB and should be that size across the vast majority of our users. Tabs can vary in size indeed, but the client code actually still assumes the old 28 KB limit [1].

[1] https://mxr.mozilla.org/mozilla-central/source/services/sync/modules/engines/tabs.js#193

> Can we log incoming post body sizes? Not sure how to do that in nginx.

That would be very helpful. Perhaps only necessary past a certain threshold. Would be good to know which client and which collection.

Comment 6

[:Atoll]

(In reply to Toby Elliott [:telliott] from comment #4)
> It occurs to me that this isn't the problem we think it is (it's a problem,
> but that's different).

I thought the problem was clients sending us 1.8MB posts.

> The only wbos that the client sends that are anywhere near 256K are tabs.
> However, a) we don't write those to the db and b) we never send more than
> one from a client. 

Regardless, we must still implement sanity limits on the incoming POSTs, even if through coincidence they end up going to memcache.

> Everything else should be well under 10k on average, no? Which suggests that
> these megapayloads are either coming to us in error, or from a different
> client.
> 
> Can we log incoming post body sizes? Not sure how to do that in nginx.

We require the sync-server to be able to turn away these requests intelligently with a valid API-correct response code, because Operations is unable to do so.

Further research into the client bug(s) potentially causing 2MB POST requests should be requested in a Client: Backend bug, rather than in this server bug.  I agree that we should do the research, but this is not the appropriate bug in which to log the results.

Comment 7

[Philipp von Weitershausen [:philikon]]

This bug feels like it's a dupe of bug 569295.

Comment 8

[Toby Elliott [:telliott]]

not quite, though they're related. This one involves legal submissions that we may still want to break up into further batches.

I'm still not sure why we're seeing 2M posts, though that's an orthogonal question to this bug

Comment 9

[:Atoll]

(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> This bug feels like it's a dupe of bug 569295.

Nope, we have to fix the server to resolve the impact of Firefox 3.6, 4, 5, 6, 7, 8, 9, 10, 11, 12, and so forth, since bug 569295 has remained unfixed and unprioritized.

What is the current timeline for fixing this on the server?

Comment 10

[Ryan Kelly [:rfkelly]]

Attachment: patch limiting size of batch uploads

Here's my first attempt at this.  You can now set configuration settings "batch_max_bytes" and "batch_max_count" as hard limits on the amount of data processed in a batch upload.  They default to 1MB and 100 items respectively.

Comment 11

[Toby Elliott [:telliott]]

Comment on attachment 584695 [details] [diff] [review]
patch limiting size of batch uploads

Most of this is great, but I think you go a little too far astray from the spec. We don't limit the number of items that you can submit in a request (though, yeah, that should be in the spec, and will be in 2.0). So, if you submit 500 valid items, we batch them up into 5 batches of 100 and do those queries. 

The reality is that the FF client only ever sends 100 at a time, so this is mostly moot, but I think it'd be best if you pulled that chunk of code and kept it in your back pocket in case things continued to go badly.

The rest looks good.

Comment 12

[Toby Elliott [:telliott]]

Comment on attachment 584695 [details] [diff] [review]
patch limiting size of batch uploads

Talking further with atoll, ops would like a max total wbos submitted too, so that is fine. Revising to r+

Comment 13

[:Atoll]

Comment on attachment 584695 [details] [diff] [review]
patch limiting size of batch uploads

>diff --git a/syncstorage/controller.py b/syncstorage/controller.py
>index cc9261a..5d5c593 100644

>+                res['failed'][item_id] = ['too many wbos for a single request']

>+                res['failed'][item_id] = ['too much data for a single request']

Please use no more than a few bytes for each of these messages. 'retry_wbo' and 'retry_bytes' would be fine, or numeric codes would also be fine (since afaik the spec leaves the reason field undefined/unparsed, so we can do whatever).

Comment 14

[Ryan Kelly [:rfkelly]]

Committed with atoll's revisions in http://hg.mozilla.org/services/server-storage/rev/20775ef714ca

Re: batching, note also that you can configure the batch_size and batch_max_count independently.  So e.g. we could process a maximum of 500 objects but save them in batches of 100.