gateway: compress frontend input payloads#6745
Conversation
crazy-max
force-pushed
the
inputs-grpc-gzip
branch
2 times, most recently
from
8087784 to
c58cec8
Compare
crazy-max
force-pushed
the
inputs-grpc-gzip
branch
from
c58cec8 to
0f29d7a
Compare
| i.mu.Lock() | ||
| i.calls++ | ||
| for _, opt := range opts { | ||
| if compressor, ok := opt.(grpc.CompressorCallOption); ok && compressor.CompressorType == grpcgzip.Name { | ||
| i.compressed = true | ||
| } | ||
| } | ||
| i.mu.Unlock() |
There was a problem hiding this comment.
I think just using an atomic int is probably easier here.
| } | ||
|
|
||
| resp, err := c.client.Inputs(ctx, &pb.InputsRequest{}) | ||
| resp, err := c.client.Inputs(ctx, &pb.InputsRequest{}, grpc.UseCompressor(gzip.Name)) |
There was a problem hiding this comment.
I assume that if the server doesn't have the capability to do the compression that this just falls back and operates correctly?
There was a problem hiding this comment.
Good catch. grpc-go does not negotiate this down if the server cannot decompress the request. It returns codes.Unimplemented before the handler runs with grpc: Decompressor is not installed for grpc-encoding "gzip" 🙈
Maybe we could have a fallback for that exact error so newer frontends retry Inputs and Solve without compression when talking to an older buildkitd that has not registered gzip? 🤔
That only preserves compatibility for payloads that already fit uncompressed. Large frontend input fan-in payloads can still hit the old max-message behavior against older daemons, which is expected.
There was a problem hiding this comment.
Added a gateway gzip capability
crazy-max
force-pushed
the
inputs-grpc-gzip
branch
from
0f29d7a to
a8a7dbe
Compare
Signed-off-by: CrazyMax <[email protected]>
crazy-max
force-pushed
the
inputs-grpc-gzip
branch
from
a8a7dbe to
bbf6b27
Compare
tonistiigi
left a comment
tonistiigi
left a comment
There was a problem hiding this comment.
I don't think this works like this. Both of these caps requests are checking the version of the daemon but in reality, it is the client talking with the frontend here, and both of the need to agree. Daemon supporting this feature and client setting it, doesn't mean that frontend know how to read it out.
Additionally this is only a stopgap that gives a little bit more room, similar of what we would get by just increasing the limits.
If we do add new frontend cap, then the cap can be something that says that inputs have been deduplicated, meaning the matching parts of the LLB branches have been removed and on load they can be looked up from other inputs.
Client can then do something like this:
- check if there are multiple inputs and if deduplicating would win significant space (to not error for missing cap for no reason)
- deduplicate and make request with the cap
- if frontend doesn't support cap then it will return typed error
- client can retry with old duplicated format and hope to not hit limits.
3 participants
fixes #6726
The gateway server now registers gzip decompression support and advertises a gateway gzip capability. The gateway client uses gzip for
Inputscalls and forSolvecalls withFrontendInputsonly when that capability is present.Large frontend input fan-in can duplicate LLB definition bytes enough to hit gRPC message limits. Gzip reduces that wire size while the capability gate keeps newer clients compatible with older daemons.