max_batch_total_size_bytes can not be correctly implemented at server side?

[ukai]

re: buildfarm/buildfarm#2366

max_batch_total_size_bytes can not be correctly implemented at server side?
Should we redefine it as maximum size of BatchUpdateBlobs request message?

[mostynb]

Should we redefine it as maximum size of BatchUpdateBlobs request message?

Yes.

[ukai]

If so, how to handle compressed upload? BatchUpdateBlobs request size may be small with compression, but uncompressed data might be too big?

[mostynb]

I assume that you are not using https://pkg.go.dev/google.golang.org/grpc#UseCompressor to compress the entire message, but are instead populating the BatchUpdateBlobsRequest message with compressed data and setting compressor field.

If so, you can estimate a reasonable upper limit for the size of the serialised protobuf message by summing up the sizes of the compressed data blobs, and adding a small overhead for the other fields in each BatchUpdateBlobsRequest.Request.

[ukai]

No, compression in BatchUpdateBlobRequest_Request's compression
https://github.com/bazelbuild/remote-apis/blob/main/build/bazel/remote/execution/v2/remote_execution.proto#L1810

[ukai]

Actually, we accidentally set limit to BatchUpdateBlob's request size, and when we enable batch upload compression, Google RBE failed due to exceed limits for sum of size_bytes of blobs. So, we realized that max_batch_total_size_bytes says blob's size, not request size. But if we fixed that, we hit another issue with buildfarm.

[mostynb]

What I have done in the past is:

1. Choose a safe upper limit for the client to build the BatchUpdateBlobs message (eg 3M if the server advertises 4M)
2. Loop over the compressed blobs that I want to upload, adding them to BatchUpdateBlobRequest and keep a running total of the compressed blob size.
3. If you reach a point where adding the next blob would exceed the upper limit (3M in this example), make the BatchUpdateBlobs request and start a new BatchUpdateBlobRequest with this blob.

You can either choose a safe upper limit (3M in this example) or use a more accurate estimate of the size required to encode each compressed blob that you add (and end up with request message sizes closer to the server's limit).

[ukai]

That was almost similar of our initial implementation, but it caused error with Google RBE with compression. i.e. BatchUpdateBlobs message size is smaller than max_batch_total_size_bytes, but uncompressed blob's size_bytes exceeds max_batch_total_size_bytes.

[mostynb]

That sounds like it might be a bug in RBE (or it is implemented strictly to the incorrect wording in the REAPI spec).

[ukai]

We think RBE implements strictly to the wording in the REAPI spec?
https://github.com/bazelbuild/remote-apis/blob/main/build/bazel/remote/execution/v2/remote_execution.proto#L2231

Maximum total size of blobs to be uploaded/downloaded using batch methods.

[mostynb]

Here's what I think this field was intended to mean: #345

[TheGrizzlyDev]

Aren't we conflating issues though? There are 2 limits here:

• the max size of the sum of all blobs in a batch request
• the max size of a grpc message

And #345 makes this a bit more confusing by implying that max_batch_total_size_bytes is applicable only to batch requests, which isn't quite accurate. The 2 limtis are distinct and the client must enforce both.

Say max_batch_total_size_bytes is 2mb then a 3mb request won't succeed regardless of the 4mb size limit

max_batch_total_size_bytes can not be correctly implemented at server side?

I believe this not to be true in fact. It's just that both limits are applicable and in your example the request failed the grpc message size limit. Wether they are correctly implemented or not is down to the implementation, but I am not sure the spec is at fault here by creating something that is inheritly impossible to implement. On the other hand adding a max_grpc_message_size capability and leaving max_batch_total_size_bytes unaltered might make more sense. To me it looks like the problem here isn't the incorrect meaning of the current capabilities, but the hidden limit to message size in gRPC. What do you think?

[mostynb]

Aren't we conflating issues though? There are 2 limits here:

• the max size of the sum of all blobs in a batch request
• the max size of a grpc message

Yes, because the spec currently only provides a way to advertise one limit, and the max size of a gRPC message is a hard upper limit:

• You can't inline blobs whose sizes together are larger than the message size limit while staying under the message size limit.
• You can specify blobs with sizes that sum up to less than the message size limit, while creating a message over the size limit. @werkt gave an example of this in BatchUpdateBlobs can't receive max_batch_total_size_bytes buildfarm/buildfarm#2366

And #345 makes this a bit more confusing by implying that max_batch_total_size_bytes is applicable only to batch requests, which isn't quite accurate. The 2 limtis are distinct and the client must enforce both.

Say max_batch_total_size_bytes is 2mb then a 3mb request won't succeed regardless of the 4mb size limit

max_batch_total_size_bytes can not be correctly implemented at server side?

I believe this not to be true in fact. It's just that both limits are applicable and in your example the request failed the grpc message size limit. Wether they are correctly implemented or not is down to the implementation, but I am not sure the spec is at fault here by creating something that is inheritly impossible to implement. On the other hand adding a max_grpc_message_size capability and leaving max_batch_total_size_bytes unaltered might make more sense. To me it looks like the problem here isn't the incorrect meaning of the current capabilities, but the hidden limit to message size in gRPC. What do you think?

Do you have a server implementation that needs to specify both, as separate limits? If so, could you explain the use case a bit? In particular, why do you need to limit the total size of the uploaded blobs in a single BatchUpdateBlobs call?

[TheGrizzlyDev]

Do you have a server implementation that needs to specify both, as separate limits? If so, could you explain the use case a bit? In particular, why do you need to limit the total size of the uploaded blobs in a single BatchUpdateBlobs call?

Say you have N small files where N is a considarably large number and the underlying storage for blobs has no batch API of its own, then we would be forcing a considerable amount of load on a single instance and we would not be able to share the work fairly amongst the cluster, which may worsen performance. Here we could argue that a better set of limits would be: max-grpc-message-size and max-blobs-in-batch rather than limiting the batch's size indirectly. In general though this is an API that allows you to state the capacity of a RBE server and its underlying system to process a batch of a given size, which cannot be inferred from the max-grpc-message-size which would also be shared with other messages. Making the size asserted that of the message instead of the blobs payload does make the limit easier to compute though, but then that's no more useful than the current limit. Imo, we should:

• limit the number of blobs in a request and not their total size or message size
• maybe find a way to tell the client about the max-grpc-message-size

Wdyt?

[mostynb]

Do you have a server implementation that needs to specify both, as separate limits? If so, could you explain the use case a bit? In particular, why do you need to limit the total size of the uploaded blobs in a single BatchUpdateBlobs call?

Say you have N small files where N is a considarably large number and the underlying storage for blobs has no batch API of its own, then we would be forcing a considerable amount of load on a single instance and we would not be able to share the work fairly amongst the cluster, which may worsen performance. Here we could argue that a better set of limits would be: max-grpc-message-size and max-blobs-in-batch rather than limiting the batch's size indirectly. In general though this is an API that allows you to state the capacity of a RBE server and its underlying system to process a batch of a given size, which cannot be inferred from the max-grpc-message-size which would also be shared with other messages. Making the size asserted that of the message instead of the blobs payload does make the limit easier to compute though, but then that's no more useful than the current limit.

Thanks- sounds reasonable.

Imo, we should:

• limit the number of blobs in a request and not their total size or message size
• maybe find a way to tell the client about the max-grpc-message-size

Wdyt?

I agree, except for the placement of the "maybe" in those two points :)

IMO announcing the max-grpc-message-size is most important because that's a hard(er) limit. We should use max_batch_total_size_bytes for this purpose since the current description isn't really useful and I suspect this may have been the original intention (though we should rename it). And we should state that it applies to all messages not just BatchUpdateBlobs. I think this would be a clear improvement over the current setup.

And we should consider making it possible to announce additional limits or hints like the one you describe (max-blobs-per-request) as separate issues.