remote: ensure that client connection is not established twice #2501
Conversation
Because remote driver implements Info() by calling Client() internally, two instances on Client are created backed by separate TCP connection. This hack avoids it and improves performance. Signed-off-by: Tonis Tiigi <[email protected]>
| client.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { | ||
| return d.Dial(ctx) | ||
| }), | ||
| client.WithTracerDelegate(delegated.DefaultExporter), |
There was a problem hiding this comment.
This is pretty 😬 , normally called by DriverHandle. Mostly weird as other drivers don't need to call it.
There was a problem hiding this comment.
But do not client.WithTracerDelegate already set as opt in:
buildx/driver/remote/driver.go
Line 80 in cfef22d
Line 167 in cfef22d
There was a problem hiding this comment.
This is how it works in regular mode and for other drivers. But for remote driver the call path can be that Info() is called that internally calls Client() (and that client is now cached). In that case no delegate is set up.
Maybe a possible solution would be to move ClientOpt as part of InitConfig
There was a problem hiding this comment.
Maybe a possible solution would be to move
ClientOptas part ofInitConfig
Ah right yeah that would be neat
There was a problem hiding this comment.
This still would mean that every driver would need to handle the ClientOpt and if they don't then at least the tracing would not work.
Because remote driver implements Info() by calling Client() internally, two instances on Client are created backed by separate TCP connection. This hack avoids it and improves performance.