[THRIFT-5453] go: NewTSocketConf should not call net.ResolveTCPAddr

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 0.14.2
• Fix Version/s: 0.15.0
• Component/s: Go - Library
• Assignee: Yuxuan Wang
• Reporter: Yuxuan Wang
• Created: 09/Aug/21 20:57
• Updated: 11/Sep/21 12:30
• Resolved: 11/Aug/21 18:14

Description

Currently we call net.ResolveTCPAddr when creating a TSocket: https://github.com/apache/thrift/blob/f7e6c654bde5d9832bede2b48b460c3e1bbbbb92/lib/go/thrift/socket.go#L53

There are two issues with this approach:

1. As the doc of net.ResolveTCPAddr explicitly called out that "The address parameter can use a host name, but this is not recommended, because it will return at most one of the host name's IP addresses." (this is probably not a big issue as it's a connection, not a connection pool)
2. More importantly, when we use a host name here, net.ResolveTCPAddr will do a DNS resolution and there's not any timeout attached to it (it's unbounded)

We should preserve the hostname here, and rely on TSocket.Open (https://github.com/apache/thrift/blob/f7e6c654bde5d9832bede2b48b460c3e1bbbbb92/lib/go/thrift/socket.go#L164-L168) to do the DNS resolution as we already have timeout attached there.

Issue Links

• links to: GitHub Pull Request #2435