nsq_to_file: --topic-pattern causes connection leaks

[jxskiss]

ready for review

[jxskiss]

Improvement:
Initialize http client only when topicsFromNSQLookupd = true, that is --topic not provided and --topic-pattern provided.

[mreiferson]

I don't think it is necessary to avoid initializing it, seems harmless and unnecessarily complex

[jxskiss]

In case of --topic provided, the connection is never used but never released, if one use nsq-to-file to read messages from many individual topics, then there will be many these connections.

[mreiferson]

You're suggesting that clusterinfo.New makes an HTTP request just by initializing it?

[jxskiss]

Ah, may be I'm understanding http/transport wrong.
I'am not sure that will these codes in "nsq/internal/http_api/api_request.go" make a connection or the connection is only made when sending the exact http endpoint request?
Does the initialization of http client/transport require resource from OS?

// A custom http.Transport with support for deadline timeouts
func NewDeadlineTransport(connectTimeout time.Duration, requestTimeout time.Duration) *http.Transport {
	transport := &http.Transport{
		Dial: func(netw, addr string) (net.Conn, error) {
			c, err := net.DialTimeout(netw, addr, connectTimeout)
			if err != nil {
				return nil, err
			}
			return &deadlinedConn{connectTimeout, c}, nil
		},
		ResponseHeaderTimeout: requestTimeout,
	}
	return transport
}

type Client struct {
	c *http.Client
}

func NewClient(tlsConfig *tls.Config, connectTimeout time.Duration, requestTimeout time.Duration) *Client {
	transport := NewDeadlineTransport(connectTimeout, requestTimeout)
	transport.TLSClientConfig = tlsConfig
	return &Client{
		c: &http.Client{
			Transport: transport,
			Timeout:   requestTimeout,
		},
	}
}

[mreiferson]

Connections should only be made when requests are made.

[jxskiss]

Thank your for the clarification, I will push a new commit soon later.

[ploxiln]

Just to clarify: the Transport and Client are just some structs with configuration (and function pointers etc), they just use a bit of memory (maybe a kilobyte or two). No file descriptors are created (or bound or connected) until a Dial or Request method is called.

[jxskiss]

The lazy initializing of clusterInfo has been reset.

[mreiferson]

LGTM