You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by yu...@apache.org on 2021/08/11 18:08:57 UTC
[thrift] branch master updated: THRIFT-5453: Defer DNS from
NewTSocketConf to TSocket.Open
This is an automated email from the ASF dual-hosted git repository.
yuxuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new fb539ae THRIFT-5453: Defer DNS from NewTSocketConf to TSocket.Open
fb539ae is described below
commit fb539ae41297c369439bea6edf39186f92f408dc
Author: Yuxuan 'fishy' Wang <yu...@reddit.com>
AuthorDate: Mon Aug 9 14:27:48 2021 -0700
THRIFT-5453: Defer DNS from NewTSocketConf to TSocket.Open
Client: go
We used to do DNS lookups in NewTSocketConf, without any timeout checks.
Stop doing that and do DNS lookups in TSocket.Open instead, which
already checks for ConnectTimeout set in TConfiguration.
Also remove the error return from NewTSocketConf.
---
CHANGES.md | 2 ++
go.sum | 3 +++
lib/go/thrift/configuration.go | 2 +-
lib/go/thrift/socket.go | 27 +++++++++++++++++----------
test/go/genmock.sh | 2 +-
test/go/src/bin/stress/main.go | 28 +++++++++++++---------------
test/go/src/common/client.go | 10 ++++++----
7 files changed, 43 insertions(+), 31 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 5ab945b..3932627 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,12 +8,14 @@
- [THRIFT-5347](https://issues.apache.org/jira/browse/THRIFT-5347) - Haskell support dropped
- [THRIFT-5381](https://issues.apache.org/jira/browse/THRIFT-5381) - possible collisions at VOID type with some 3rd-party libraries on Haxe cpp targets
- [THRIFT-5396](https://issues.apache.org/jira/browse/THRIFT-5396) - deprecate netstd "Async" method postfix
+- [THRIFT-5453](https://issues.apache.org/jira/browse/THRIFT-5453) - go: NewTSocketConf no longer returns an error
### Go
- [THRIFT-5404](https://issues.apache.org/jira/browse/THRIFT-5404) - TTransportException.Timeout would correctly return true when it's connect timeout during TSocket.Open call
- [THRIFT-5389](https://issues.apache.org/jira/browse/THRIFT-5389) - The compiler now generates correct go code with thrift constant defined on optional enum/typedef fields
- [THRIFT-4797](https://issues.apache.org/jira/browse/THRIFT-4797) - The compiler now correctly auto renames import thrift namespaces when they collide with system imports
+- [THRIFT-5453](https://issues.apache.org/jira/browse/THRIFT-5453) - Defer DNS lookups from NewTSocketConf (without any timeout check) to TSocket.Open (subject to ConnectTimeout set in TConfiguration)
## 0.14.2
diff --git a/go.sum b/go.sum
index 646b11a..cfde584 100644
--- a/go.sum
+++ b/go.sum
@@ -2,6 +2,7 @@ github.com/golang/mock v1.5.0 h1:jlYHihg//f7RRwuPfptm04yp4s7O6Kw8EZiVYIGcH0g=
github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
@@ -9,6 +10,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e h1:aZzprAO9/8oim3qStq3wc1Xuxx4QmAGriC4VU4ojemQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/lib/go/thrift/configuration.go b/lib/go/thrift/configuration.go
index 454d9f3..de27edd 100644
--- a/lib/go/thrift/configuration.go
+++ b/lib/go/thrift/configuration.go
@@ -56,7 +56,7 @@ const (
//
// For example, say you want to migrate this old code into using TConfiguration:
//
-// sccket := thrift.NewTSocketTimeout("host:port", time.Second)
+// sccket, err := thrift.NewTSocketTimeout("host:port", time.Second, time.Second)
// transFactory := thrift.NewTFramedTransportFactoryMaxLength(
// thrift.NewTTransportFactory(),
// 1024 * 1024 * 256,
diff --git a/lib/go/thrift/socket.go b/lib/go/thrift/socket.go
index 0cf59a0..eeac4f1 100644
--- a/lib/go/thrift/socket.go
+++ b/lib/go/thrift/socket.go
@@ -29,16 +29,26 @@ type TSocket struct {
conn *socketConn
addr net.Addr
cfg *TConfiguration
+}
+
+// tcpAddr is a naive implementation of net.Addr that does nothing extra.
+type tcpAddr string
+
+var _ net.Addr = tcpAddr("")
+
+func (ta tcpAddr) Network() string {
+ return "tcp"
+}
- connectTimeout time.Duration
- socketTimeout time.Duration
+func (ta tcpAddr) String() string {
+ return string(ta)
}
// Deprecated: Use NewTSocketConf instead.
func NewTSocket(hostPort string) (*TSocket, error) {
return NewTSocketConf(hostPort, &TConfiguration{
noPropagation: true,
- })
+ }), nil
}
// NewTSocketConf creates a net.Conn-backed TTransport, given a host and port.
@@ -49,12 +59,8 @@ func NewTSocket(hostPort string) (*TSocket, error) {
// ConnectTimeout: time.Second, // Use 0 for no timeout
// SocketTimeout: time.Second, // Use 0 for no timeout
// })
-func NewTSocketConf(hostPort string, conf *TConfiguration) (*TSocket, error) {
- addr, err := net.ResolveTCPAddr("tcp", hostPort)
- if err != nil {
- return nil, err
- }
- return NewTSocketFromAddrConf(addr, conf), nil
+func NewTSocketConf(hostPort string, conf *TConfiguration) *TSocket {
+ return NewTSocketFromAddrConf(tcpAddr(hostPort), conf)
}
// Deprecated: Use NewTSocketConf instead.
@@ -64,7 +70,7 @@ func NewTSocketTimeout(hostPort string, connTimeout time.Duration, soTimeout tim
SocketTimeout: soTimeout,
noPropagation: true,
- })
+ }), nil
}
// NewTSocketFromAddrConf creates a TSocket from a net.Addr
@@ -172,6 +178,7 @@ func (p *TSocket) Open() error {
msg: err.Error(),
}
}
+ p.addr = p.conn.RemoteAddr()
return nil
}
diff --git a/test/go/genmock.sh b/test/go/genmock.sh
index bccfdf3..27cd0c4 100644
--- a/test/go/genmock.sh
+++ b/test/go/genmock.sh
@@ -9,4 +9,4 @@ GO111MODULE=on go install -mod=mod github.com/golang/mock/mockgen
`go env GOPATH`/bin/mockgen -build_flags "-mod=mod" -destination=src/common/mock_handler.go -package=common github.com/apache/thrift/test/go/src/gen/thrifttest ThriftTest
-rm -Rf $GOPATH
+chmod a+w -R $GOPATH && rm -Rf $GOPATH
diff --git a/test/go/src/bin/stress/main.go b/test/go/src/bin/stress/main.go
index 3ff0a39..9f32676 100644
--- a/test/go/src/bin/stress/main.go
+++ b/test/go/src/bin/stress/main.go
@@ -158,13 +158,11 @@ func main() {
}
func client(protocolFactory thrift.TProtocolFactory) {
- trans, err := thrift.NewTSocket(hostPort)
- if err != nil {
- log.Fatalf("Unable to create server socket: %s", err)
- }
+ ctx := context.Background()
+ trans := thrift.NewTSocketConf(hostPort, nil)
btrans := thrift.NewTBufferedTransport(trans, 2048)
client := stress.NewServiceClientFactory(btrans, protocolFactory)
- err = trans.Open()
+ err := trans.Open()
if err != nil {
log.Fatalf("Unable to open connection: %s", err)
}
@@ -173,45 +171,45 @@ func client(protocolFactory thrift.TProtocolFactory) {
switch callType {
case echoVoid:
for i := 0; i < *loop; i++ {
- client.EchoVoid()
+ client.EchoVoid(ctx)
atomic.AddInt64(&clicounter, 1)
}
case echoByte:
for i := 0; i < *loop; i++ {
- client.EchoByte(42)
+ client.EchoByte(ctx, 42)
atomic.AddInt64(&clicounter, 1)
}
case echoI32:
for i := 0; i < *loop; i++ {
- client.EchoI32(4242)
+ client.EchoI32(ctx, 4242)
atomic.AddInt64(&clicounter, 1)
}
case echoI64:
for i := 0; i < *loop; i++ {
- client.EchoI64(424242)
+ client.EchoI64(ctx, 424242)
atomic.AddInt64(&clicounter, 1)
}
case echoString:
for i := 0; i < *loop; i++ {
- client.EchoString("TestString")
+ client.EchoString(ctx, "TestString")
atomic.AddInt64(&clicounter, 1)
}
case echiList:
l := []int8{-10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8}
for i := 0; i < *loop; i++ {
- client.EchoList(l)
+ client.EchoList(ctx, l)
atomic.AddInt64(&clicounter, 1)
}
case echoSet:
- s := map[int8]struct{}{-10: {}, -9: {}, -8: {}, -7: {}, -6: {}, -5: {}, -4: {}, -3: {}, -2: {}, -1: {}, 0: {}, 1: {}, 2: {}, 3: {}, 4: {}, 5: {}, 6: {}, 7: {}, 8: {}}
+ s := []int8{-10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8}
for i := 0; i < *loop; i++ {
- client.EchoSet(s)
+ client.EchoSet(ctx, s)
atomic.AddInt64(&clicounter, 1)
}
case echoMap:
m := map[int8]int8{-10: 10, -9: 9, -8: 8, -7: 7, -6: 6, -5: 5, -4: 4, -3: 3, -2: 2, -1: 1, 0: 0, 1: 1, 2: 2, 3: 3, 4: 4, 5: 5, 6: 6, 7: 7, 8: 8}
for i := 0; i < *loop; i++ {
- client.EchoMap(m)
+ client.EchoMap(ctx, m)
atomic.AddInt64(&clicounter, 1)
}
}
@@ -245,7 +243,7 @@ func (h *handler) EchoList(ctx context.Context, arg []int8) (r []int8, err error
atomic.AddInt64(&counter, 1)
return arg, nil
}
-func (h *handler) EchoSet(ctx context.Context, arg map[int8]struct{}) (r map[int8]struct{}, err error) {
+func (h *handler) EchoSet(ctx context.Context, arg []int8) (r []int8, err error) {
atomic.AddInt64(&counter, 1)
return arg, nil
}
diff --git a/test/go/src/common/client.go b/test/go/src/common/client.go
index 15973d8..2015035 100644
--- a/test/go/src/common/client.go
+++ b/test/go/src/common/client.go
@@ -62,15 +62,17 @@ func StartClient(
return nil, nil, fmt.Errorf("Invalid protocol specified %s", protocol)
}
if debugClientProtocol {
- protocolFactory = thrift.NewTDebugProtocolFactory(protocolFactory, "client:")
+ protocolFactory = thrift.NewTDebugProtocolFactoryWithLogger(protocolFactory, "client:", thrift.StdLogger(nil))
}
if ssl {
- trans, err = thrift.NewTSSLSocket(hostPort, &tls.Config{InsecureSkipVerify: true})
+ trans, err = thrift.NewTSSLSocketConf(hostPort, &thrift.TConfiguration{
+ TLSConfig: &tls.Config{InsecureSkipVerify: true},
+ })
} else {
if domain_socket != "" {
- trans, err = thrift.NewTSocket(domain_socket)
+ trans = thrift.NewTSocketConf(domain_socket, nil)
} else {
- trans, err = thrift.NewTSocket(hostPort)
+ trans = thrift.NewTSocketConf(hostPort, nil)
}
}
if err != nil {