[CCCLL]

DnsSrvHostProvider receives the client's ZKClientConfig, and ZKClientConfig copies zookeeper.hostProvider.dnsSrvRefreshIntervalSeconds from the system properties like the other client settings, but the refresh interval was read with Long.getLong(). A value set in a client config file or with ZKClientConfig.setProperty() was ignored, so the provider kept refreshing every 60 seconds (or used whatever the system property said).

The interval is now read from the given ZKClientConfig, falling back to a new ZKClientConfig when none is passed, the same way HostConnectionManager does. The system property keeps working because ZKClientConfig is initialized from it.

Behaviour changes, both limited to this new 3.10.0 provider:

• A value in the client config now takes precedence over the system property, as it does for the other client properties.
• A non-numeric value used to be silently replaced by the 60 second default; it now fails the provider construction with a NumberFormatException, consistent with how a negative value is rejected and how zookeeper.request.timeout is handled.

Testing

• New DnsSrvHostProviderTest#testRefreshIntervalFromClientConfig (interval 1 in ZKClientConfig, expects a periodic lookup) and #testRefreshIntervalNegativeInClientConfig (-1 in ZKClientConfig is rejected) fail without the fix and pass with it. The existing system property tests still pass.
• org.apache.zookeeper.client.*Test, HostProviderSelectionTest, StaticHostProviderTest, ConnectStringParserTest and CustomHostProviderTest pass; checkstyle and spotbugs report no issues.
• End to end against a standalone server and a local DNS responder serving the SRV record: a client created with new ZooKeeper("dns-srv://...", ..., new ZKClientConfig(configFile)), where the file sets the interval to 1, sent 1 SRV query during a 3.5 second session before the fix and 4 (one per second) after it.

[kezhuw]

I think the problem resides in constructor of ZooKeeper. We should give clientConfig a no null ZKClientConfig there to not misuse it. But the change is good anyway.

zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java

Lines 1139 to 1147 in 777b372

	ZKClientConfig clientConfig = options.getClientConfig();
	this.clientConfig = clientConfig != null ? clientConfig : new ZKClientConfig();
	ConnectStringParser connectStringParser = new ConnectStringParser(connectString);
	HostProvider hostProvider;
	if (options.getHostProvider() != null) {
	hostProvider = options.getHostProvider().apply(connectStringParser.getServerAddresses());
	} else {
	hostProvider = HostProviderFactory.create(connectStringParser, clientConfig);
	}

[kezhuw]

I would prefer to initialize clientConfig to non null here so we don't make mistake again.

[CCCLL]

Done in 0ebc4a9: clientConfig is replaced with a new ZKClientConfig right where it is read from the options, so the rest of the constructor, including the HostProviderFactory call, only sees the non-null one.

[anmolnar]

lgtm. Please address remaining comment.