apache
[CURATOR-570] Excessive calls to ZooKeeper.updateServerList (which can result in session death)
On suspend and reconnect, Curator calls ZooKeeper.updateServerList via ConnectionState.checkState --> ConnectionState.handleNewConnectionString. In addition, recipes may be triggered by this as well, and they too make calls ZooKeeper.updateServerList via ConnectState.checkTimeouts --> ConnectionState.handleNewConnectionString.
This happens even though the connection string has not actually changed.
Due to ZOOKEEPER-3825, this can cause the connection to be closed immediately. On its own this would be perceived as a glitch. But due to the Curator-induced calls, what we see is a cycle of SUSPENDED/RECONNECTED, until eventually the session dies and a new session is recreated.
Based on the source code (at time of writing), ZooKeeper.updateServerList is not intended to be called frequently like this.
Originally reported by ryarran, imported from: Excessive calls to ZooKeeper.updateServerList (which can result in session death)
- status: Open
- priority: Major
- resolution: Unresolved
- imported: 2025-01-21
Whether or not we change this in Curator, you can add your own EnsembleProvider that does not do this. In CuratorFrameworkFactory, use this method instead of connectString(String connectString):
CuratorFrameworkFactory.builder()
....
.ensembleProvider(new FixedEnsembleProvider(connectionString, false)) // false for updateServerListEnabled
....
Rhys Yarranton, I was just taking a quick look at this. Maybe I'm missing something, but from my reading getNewConnectionString() will only return non null if the existing connection string is non null and the connection string provided by the ensemble is different from the existing connection string.
String getNewConnectionString() { String helperConnectionString = (helper != null) ? helper.getConnectionString() : null; String ensembleProviderConnectionString = ensembleProvider.getConnectionString(); return ((helperConnectionString != null) && !ensembleProviderConnectionString.equals(helperConnectionString)) ? ensembleProviderConnectionString : null; }
and handleNewConnection only gets called if getNewConnectionString() returns a non null value.
Cam raises a good question. I just put a debugger on a local test program and found helperConnectionString and ensembleProviderConnectionString had superficially different values, one using host names and the other using IP addresses. Looking further, disagreement can also happen due servers being in a different order. (Which in our case appears to be possible, even likely.)
I also notice that the 4.2 and 4.3 code for getNewConnectionString are different. Here is the 4.2 version:
String getNewConnectionString() { String helperConnectionString = (helper != null) ? helper.getConnectionString() : null; return ((helperConnectionString != null) && !ensembleProvider.getConnectionString().equals(helperConnectionString)) ? helperConnectionString : null; }
Our worst problems were under 4.2, so it's possible the difference is significant.
Update: The 4.2 vs 4.3 diff is part of CURATOR-551.
Thanks Rhys Yarranton, I must have missed that you were on 4.2. The previous implementation was fixed as part of CURATOR-551 as you've suggested. It looks like the naive comparison is still going to be an issue though. The ordering is a simple a enough fix, but the IP address vs hostname is more of a pain. I'm not that familiar with how Zookeeper decides how to return the ensemble connection string. Do you know under which conditions it returns a hostname and which it returns an IP address? I would have thought it should be consistent across calls.
In zk 3.5.5, the value coming fr/ the zk server could be either hostname or IP address. (I suspect based on what flavour went into the zk server config.) On the zk client side, this gets turned into socket address in QuorumPeer(long, String) using new InetSocketAddress(String, int). This will attempt to resolve the address, and so could have both. (This is interesting, as in some other places zk uses InetSocketAddress.createUnresolved, and createUnresolved would seem appropriate here.)
Then it gets to EnsembleTracker.configToConnectionString (this is Curator code now), which creates a String. The address part is taken as:
hostAddress = server.clientAddr.getAddress().getHostAddress();
which will be the IP address. Or if the address failed to resolve, it could throw an NPE.
Thanks Rhys Yarranton, this should at least mean that the representation of each host is consistent (either IP or hostname), with a few possible edge cases (hostname going from resolvable to unresolvable, ZK config changing from hostname to IP), which can maybe be ignored. If we just changed the logic so that it splits the hostname string into a Set and then does a comparison on that, that should stop at least the vast majority of unnecessary calls to updateServerList
We need to be very careful about converting between hosts/IPs as these can be very expensive operations.
Jordan Zimmerman, agreed, but this is already done in the EnsembleTracker. I'm just suggesting that we store a Set of all the hostnames / IPs in addition to the string itself, and use the Set to do the comparison to determine if it's changed. That way we can handle ZooKeeper returning the same set of hosts / IPs in a different order.
I'd like to see this all addressed. So, yeah, let's do it. The EnsembleTracker is odd - I don't really understand it tbh but I think it gets called way too much. Do you have time to do this Cam McKenzie or Rhys Yarranton?
I won't have a chance to look this week, but I will try and get onto it next week. Would be good to have a unit test that reproduces the issue to start with.
Rhys Yarranton, do you have a unit test that can reproduce this? I've been trying to write one, but I can't reproduce the problem. I see that if you configure the original connection string using hostnames, then you will get a single case of calling into updateServerList() unnecessarily (as the hostnames don't match the IPs in a string comparison), but after that, it seems to work fine, as the 'current' connection string stored by Curator is the one that contains the IPs.
I'm only running on the test server, but I don't see the configured hostnames coming back in a different order either. I was only running with 3 instances in the cluster though, not sure if you were running with more which may provoke the issue.
No unit test. What you describe sounds plausible for 4.3 (i.e., given the fix for CURATOR-551). iiuc,
- At first FixedEnsembleProvider will have the connection string provided to Curator at start-up. This value should also make its way into Helper.
- After connect or reconnect or the watch fires, EnsembleTracker will determine a (possibly different) connection string according to the ZK servers based on the special config node. This is a background operation. Once determined, this value gets put into the FixedEnsembleProvider, but is not immediately pushed into Helper.
- At a later time, say connection suspended, HandleHolder will check the FixedEnsembleProvider value against the Helper value. If they differ, the FixedEnsembleProvider value will be returned. (In 4.3, not 4.2.)
- Shortly after, but in a different method (ConnectionState.handleNewConnectionString)), this new value will be put back into Helper. (So there may be a window for multiple check-fails to amass before it resolves itself, though it seems it should be small.)
Remark: ExhibitorEnsembleProvider is different from FixedEnsembleProvider for our purposes. It ignores the value from EnsembleTracker.
Is the value from EnsembleTracker consistent, apart from genuine changes? The servers are in a HashMap with a key of the server ID. There are things that could make that inconsistent, like two keys in the same bucket inserted in different orders. In practice this seems like it would be rare, maybe even never. (Of course, ZooKeeper could change it to an inconsistent implementation, not that there's any obvious reason to.)
You could guard against this if you wanted. EnsembleTracker.configToConnectionString is Curator code. It could impose an order on the servers, e.g., by interjecting a TreeMap.
So ... absent HashMap madness, and unless values are getting reset somewere I haven't noticed ... in 4.3 there could be one glitch. Or possibly one glitch detected more than once due to multiple threads. But after that the value should ultimately have come from EnsembleTracker and things should clear up.
Caveat, I do not pretend to understand this code well.
Thanks Rhys Yarranton, seems like it would make sense to modify Curator to have consistent ordering of clients. Additionally, it is probably worth preprocessing the connection string provided directly to Curator (rather than the one returned from Zookeeper), so that if Curator has been supplied hostnames rather than IP addresses, then we don't get the initial unnecessary call to updateServerList().
Jordan Zimmerman, do you know how this works on the Zookeeper side? I was just thinking that if the connection string provided to Curator is a list of hostnames, that storing them as IP addresses rather than resolving them each time may cause issues with load balancers etc. If ZK resolves the hostnames to IP addresses anyway though, then we're just delaying the inevitable by not resolving the hostnames before the initial connection to ZK.
ZooKeeper has an abstraction for this now: HostProvider - we can't assume anything about this. We really should not alter the connection string given to Curator. I'm -1 on Curator doing any modification to the connection string. TBH - that was the idea of the EnsembleProvider as I recall. If it needs improving we should do that.
Getting confused here. ZooKeeper does not give Curator a connection string. It supplies a node of configuration information. Curator reads this and constructs a connection string from it.
If you're using Exhibitor, then it's different, and ExhibitorEnsembleProvider does a lot of work checking for updates etc. But if using ZooKeeper's dynamic reconfiguration, the FixedEnsembleProvider is essentially just a holder for the connection string. The logic to decide when new connection strings are required and how to construct them lies elsewhere.
> ZooKeeper does not give Curator a connection string
It does when there's a configuration change. I probably added confusion when I discussed HostProvider. That's probably not related. My larger point is that if we get a value from ZooKeeper's config we need to make sure we use it as given.
Could you point me to what code you are looking at? The code I'm looking at (4.3) starts in EnsembleTracker, which uses the getConfig() op with a watcher to monitor the ZooKeeper config node. The ZooKeeper configuration node is not a connection string per se. In the path I'm looking at, the connection string is constructed from this configuration node data in EnsembleTracker.processConfigData and configToConnectionString.
Oh? It could be then. Sorry if I'm adding noise here. I'm not that familiar with that code.
Jordan Zimmerman, the code in question is in EnsembleTracker.
public static String configToConnectionString(QuorumVerifier data) throws Exception
This turns the QuorumVerifier data returned from Zookeeper into a connection string that is used by Curator.
There are 2 minor issues.
1.) Zookeeper always returns IPs (based on my testing, not 100% sure on this) in the config update calls. So, even if the user of Curator provides the full set of ZK nodes in the connection string, if they are provided as hostnames rather than IPs, then when we get a config event, this always results in an update to the connection string.
2.) The ordering of IP addresses may not be deterministic. I haven't seen ordering issues, by Rhys Yarranton has mentioned that he has. On the ZK side, I believe the data is stored in a map before streaming, so ordering is not guaranteed.
So, one possible solution is to
-Enforce ordering of IP addresses on the Curator side. I don't believe this has any effect on how connections to ZK are actually established. From memory, a host is chosen at random to connect to to try and distribute load.
-Execute the conversion of hostnames to IP addresses and ordering them on the initial connection string provided to Curator. For the case where the specified connection string contains all configured ZK nodes, this prevents one unnecessary call to updateServerList() for the case where the client provides hostnames and ZK returns IP addresses.
No, the only way I could vaguely reproduce the problem was by going through with a debugger. The only time I could really see it being an issue is if the order of IP addresses returned from Zookeeper changes across each iteration (with the actual content remaining the same), but at least with the test server I did not see this happening.