[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]

Shall we construct a ZKClientConfig if none provided and pass it to HostConnectionManager ? So the two see a consistent view of client config.

[CCCLL]

Thanks, done in b08df25. DnsSrvHostProvider now creates the ZKClientConfig once when none is given and hands the same instance to HostConnectionManager and the refresh interval lookup. As you suggested in the review, the ZooKeeper constructor now also passes its non-null clientConfig to HostProviderFactory, so the client and the host provider see the same config.